From bd2aae1f711898e013a216f1ebea9bba29526792 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Thu, 14 Jul 2022 11:12:52 +0200 Subject: [PATCH] Revert "ocdav: skip space lookup on spaces propfind (#2977)" This reverts commit 2a6ff114bc4b14009892c5422c32c2277e601b3e. --- .../skip-space-lookup-on-space-propfind.md | 8 - .../grpc/services/gateway/storageprovider.go | 8 +- .../services/gateway/storageprovidercache.go | 23 +- .../sharesstorageprovider.go | 48 +- .../storageprovider/storageprovider.go | 8 +- .../http/services/owncloud/ocdav/delete.go | 8 +- .../http/services/owncloud/ocdav/locks.go | 17 +- .../http/services/owncloud/ocdav/mkcol.go | 4 +- .../owncloud/ocdav/propfind/propfind.go | 444 +++++------------- .../owncloud/ocdav/propfind/propfind_test.go | 242 ++++------ .../http/services/owncloud/ocdav/proppatch.go | 46 +- .../services/owncloud/ocdav/publicfile.go | 2 +- internal/http/services/owncloud/ocdav/put.go | 7 +- .../http/services/owncloud/ocdav/report.go | 2 +- .../owncloud/ocdav/spacelookup/spacelookup.go | 12 +- .../http/services/owncloud/ocdav/versions.go | 2 +- .../storage/eoshomewrapper/eoshomewrapper.go | 8 +- pkg/cbox/storage/eoswrapper/eoswrapper.go | 10 +- pkg/rhttp/datatx/utils/download/download.go | 2 +- pkg/storage/fs/nextcloud/nextcloud.go | 5 +- pkg/storage/fs/nextcloud/nextcloud_test.go | 4 +- pkg/storage/fs/owncloudsql/owncloudsql.go | 4 +- pkg/storage/fs/s3/s3.go | 4 +- pkg/storage/storage.go | 4 +- .../utils/decomposedfs/decomposedfs.go | 30 +- .../decomposedfs_concurrency_test.go | 2 +- pkg/storage/utils/decomposedfs/node/node.go | 87 +--- .../utils/decomposedfs/node/node_test.go | 8 +- .../utils/decomposedfs/node/permissions.go | 9 +- .../utils/decomposedfs/tree/tree_test.go | 4 +- pkg/storage/utils/decomposedfs/upload_test.go | 10 +- pkg/storage/utils/eosfs/eosfs.go | 14 +- pkg/storage/utils/eosfs/spaces.go | 2 +- pkg/storage/utils/localfs/localfs.go | 4 +- .../expected-failures-on-OCIS-storage.md | 13 + .../expected-failures-on-S3NG-storage.md | 13 + 36 files changed, 395 insertions(+), 723 deletions(-) delete mode 100644 changelog/unreleased/skip-space-lookup-on-space-propfind.md diff --git a/changelog/unreleased/skip-space-lookup-on-space-propfind.md b/changelog/unreleased/skip-space-lookup-on-space-propfind.md deleted file mode 100644 index 63600068a2..0000000000 --- a/changelog/unreleased/skip-space-lookup-on-space-propfind.md +++ /dev/null @@ -1,8 +0,0 @@ -Enhancement: skip space lookup on space propfind - -We now construct the space id from the /dav/spaces URL intead of making a request to the registry. - -https://github.com/cs3org/reva/pull/2977 -https://github.com/owncloud/ocis/issues/1277 -https://github.com/owncloud/ocis/issues/2144 -https://github.com/owncloud/ocis/issues/3073 \ No newline at end of file diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 23dfa13e9c..1b6b3b8dcb 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -837,12 +837,7 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St }, nil } - return c.Stat(ctx, &provider.StatRequest{ - Opaque: req.Opaque, - Ref: ref, - ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, - FieldMask: req.FieldMask, - }) + return c.Stat(ctx, &provider.StatRequest{Opaque: req.Opaque, Ref: ref, ArbitraryMetadataKeys: req.ArbitraryMetadataKeys}) } func (s *svc) ListContainerStream(_ *provider.ListContainerStreamRequest, _ gateway.GatewayAPI_ListContainerStreamServer) error { @@ -863,7 +858,6 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ Opaque: req.Opaque, Ref: ref, ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, - FieldMask: req.FieldMask, }) } diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go index 978006fd32..916135446b 100644 --- a/internal/grpc/services/gateway/storageprovidercache.go +++ b/internal/grpc/services/gateway/storageprovidercache.go @@ -229,37 +229,24 @@ type cachedAPIClient struct { // generates a user specific key pointing to ref - used for statcache // a key looks like: uid:1234-1233!sid:5678-5677!oid:9923-9934!path:/path/to/source // as you see it adds "uid:"/"sid:"/"oid:" prefixes to the uuids so they can be differentiated -func statKey(user *userpb.User, ref *provider.Reference, metaDataKeys, fieldMaskPaths []string) string { +func statKey(user *userpb.User, ref *provider.Reference, metaDataKeys []string) string { if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" { return "" } - key := strings.Builder{} - key.WriteString("uid:") - key.WriteString(user.Id.OpaqueId) - key.WriteString("!sid:") - key.WriteString(ref.ResourceId.StorageId) - key.WriteString("!oid:") - key.WriteString(ref.ResourceId.OpaqueId) - key.WriteString("!path:") - key.WriteString(ref.Path) + key := "uid:" + user.Id.OpaqueId + "!sid:" + ref.ResourceId.StorageId + "!oid:" + ref.ResourceId.OpaqueId + "!path:" + ref.Path for _, k := range metaDataKeys { - key.WriteString("!mdk:") - key.WriteString(k) - } - for _, p := range fieldMaskPaths { - key.WriteString("!fmp:") - key.WriteString(p) + key += "!mdk:" + k } - return key.String() + return key } // Stat looks in cache first before forwarding to storage provider func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) { cache := c.caches[stat] - key := statKey(ctxpkg.ContextMustGetUser(ctx), in.GetRef(), in.GetArbitraryMetadataKeys(), in.GetFieldMask().GetPaths()) + key := statKey(ctxpkg.ContextMustGetUser(ctx), in.Ref, in.ArbitraryMetadataKeys) if key != "" { s := &provider.StatResponse{} if err := pullFromCache(cache, key, s); err == nil { diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index ef108120e3..2ff40af8ef 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -24,6 +24,7 @@ import ( "path/filepath" "strings" + "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/storagespace" "google.golang.org/grpc" codes "google.golang.org/grpc/codes" @@ -368,10 +369,10 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } var receivedShares []*collaboration.ReceivedShare - var shareInfo map[string]*provider.ResourceInfo + var shareMd map[string]share.Metadata var err error if fetchShares { - receivedShares, shareInfo, err = s.fetchShares(ctx) + receivedShares, shareMd, err = s.fetchShares(ctx) if err != nil { return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } @@ -388,13 +389,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora OpaqueId: utils.ShareStorageProviderID, } if spaceID == nil || isShareJailRoot(spaceID) { - earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareInfo) + earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareMd) var opaque *typesv1beta1.Opaque var mtime *typesv1beta1.Timestamp if earliestShare != nil { - if info, ok := shareInfo[earliestShare.Id.OpaqueId]; ok { - mtime = info.Mtime - opaque = utils.AppendPlainToOpaque(opaque, "etag", info.Etag) + if md, ok := shareMd[earliestShare.Id.OpaqueId]; ok { + mtime = md.Mtime + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) } } // only display the shares jail if we have accepted shares @@ -423,16 +424,20 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora // none of our business continue } + var opaque *typesv1beta1.Opaque + if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) + } // we know a grant for this resource space := &provider.StorageSpace{ + Opaque: opaque, Id: &provider.StorageSpaceId{ OpaqueId: storagespace.FormatResourceID(*root), }, SpaceType: "grant", Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // the sharesstorageprovider keeps track of mount points - Root: root, - RootInfo: shareInfo[receivedShare.Share.Id.OpaqueId], + Root: root, } res.StorageSpaces = append(res.StorageSpaces, space) @@ -460,7 +465,9 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } } var opaque *typesv1beta1.Opaque - if _, ok := shareInfo[receivedShare.Share.Id.OpaqueId]; !ok { + if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) + } else { // we could not stat the share, skip it continue } @@ -483,8 +490,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora SpaceType: "mountpoint", Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient // the sharesstorageprovider keeps track of mount points - Root: root, - RootInfo: shareInfo[receivedShare.Share.Id.OpaqueId], + Root: root, } // TODO in the future the spaces registry will handle the alias for share spaces. @@ -705,7 +711,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide PermissionSet: &provider.ResourcePermissions{ // TODO }, - Etag: shareMd[earliestShare.Id.OpaqueId].Etag, + Etag: shareMd[earliestShare.Id.OpaqueId].ETag, Owner: owner.Id, }, }, nil @@ -1025,7 +1031,7 @@ func (s *service) rejectReceivedShare(ctx context.Context, receivedShare *collab return errtypes.NewErrtypeFromStatus(res.Status) } -func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedShare, map[string]*provider.ResourceInfo, error) { +func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedShare, map[string]share.Metadata, error) { lsRes, err := s.sharesProviderClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ // FIXME filter by received shares for resource id - listing all shares is tooo expensive! }) @@ -1036,7 +1042,7 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest") } - shareMetaData := make(map[string]*provider.ResourceInfo, len(lsRes.Shares)) + shareMetaData := make(map[string]share.Metadata, len(lsRes.Shares)) for _, rs := range lsRes.Shares { // only stat accepted shares if rs.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { @@ -1057,13 +1063,13 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha Msg("ListRecievedShares: failed to stat the resource") continue } - shareMetaData[rs.Share.Id.OpaqueId] = sRes.Info + shareMetaData[rs.Share.Id.OpaqueId] = share.Metadata{ETag: sRes.Info.Etag, Mtime: sRes.Info.Mtime} } return lsRes.Shares, shareMetaData, nil } -func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareInfo map[string]*provider.ResourceInfo) (earliestShare *collaboration.Share, atLeastOneAccepted bool) { +func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd map[string]share.Metadata) (earliestShare *collaboration.Share, atLeastOneAccepted bool) { for _, rs := range receivedShares { var hasCurrentMd bool var hasEarliestMd bool @@ -1075,10 +1081,10 @@ func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareInfo // We cannot assume that every share has metadata if current.Id != nil { - _, hasCurrentMd = shareInfo[current.Id.OpaqueId] + _, hasCurrentMd = shareMd[current.Id.OpaqueId] } if earliestShare != nil && earliestShare.Id != nil { - _, hasEarliestMd = shareInfo[earliestShare.Id.OpaqueId] + _, hasEarliestMd = shareMd[earliestShare.Id.OpaqueId] } switch { @@ -1087,10 +1093,10 @@ func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareInfo // ignore if one of the shares has no metadata case !hasEarliestMd || !hasCurrentMd: continue - case shareInfo[current.Id.OpaqueId].Mtime.Seconds > shareInfo[earliestShare.Id.OpaqueId].Mtime.Seconds: + case shareMd[current.Id.OpaqueId].Mtime.Seconds > shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds: earliestShare = current - case shareInfo[current.Id.OpaqueId].Mtime.Seconds == shareInfo[earliestShare.Id.OpaqueId].Mtime.Seconds && - shareInfo[current.Id.OpaqueId].Mtime.Nanos > shareInfo[earliestShare.Id.OpaqueId].Mtime.Nanos: + case shareMd[current.Id.OpaqueId].Mtime.Seconds == shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds && + shareMd[current.Id.OpaqueId].Mtime.Nanos > shareMd[earliestShare.Id.OpaqueId].Mtime.Nanos: earliestShare = current } } diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index ce34ddcbec..63c955043d 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -672,7 +672,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro } } - md, err := s.storage.GetMD(ctx, req.Ref, []string{}, []string{"id"}) + md, err := s.storage.GetMD(ctx, req.Ref, []string{}) if err != nil { return &provider.DeleteResponse{ Status: status.NewStatusFromErrType(ctx, "can't stat resource to delete", err), @@ -718,7 +718,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide Value: attribute.StringValue(req.Ref.String()), }) - md, err := s.storage.GetMD(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) + md, err := s.storage.GetMD(ctx, req.Ref, req.ArbitraryMetadataKeys) if err != nil { return &provider.StatResponse{ Status: status.NewStatusFromErrType(ctx, "stat", err), @@ -747,7 +747,7 @@ func (s *service) ListContainerStream(req *provider.ListContainerStreamRequest, ctx := ss.Context() log := appctx.GetLogger(ctx) - mds, err := s.storage.ListFolder(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) + mds, err := s.storage.ListFolder(ctx, req.Ref, req.ArbitraryMetadataKeys) if err != nil { var st *rpc.Status switch err.(type) { @@ -792,7 +792,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer providerID := unwrapProviderID(req.Ref.GetResourceId()) defer rewrapProviderID(req.Ref.GetResourceId(), providerID) - mds, err := s.storage.ListFolder(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) + mds, err := s.storage.ListFolder(ctx, req.Ref, req.ArbitraryMetadataKeys) res := &provider.ListContainerResponse{ Status: status.NewStatusFromErrType(ctx, "list container", err), Infos: mds, diff --git a/internal/http/services/owncloud/ocdav/delete.go b/internal/http/services/owncloud/ocdav/delete.go index f28432da6d..168d398f64 100644 --- a/internal/http/services/owncloud/ocdav/delete.go +++ b/internal/http/services/owncloud/ocdav/delete.go @@ -96,7 +96,8 @@ func (s *svc) handleDelete(ctx context.Context, w http.ResponseWriter, r *http.R w.WriteHeader(http.StatusNoContent) case rpc.Code_CODE_NOT_FOUND: w.WriteHeader(http.StatusNotFound) - m := "Resource not found" // mimic the oc10 error message + // TODO path might be empty or relative... + m := fmt.Sprintf("Resource %v not found", ref.Path) b, err := errors.Marshal(http.StatusNotFound, m, "") errors.HandleWebdavError(&log, w, b, err) case rpc.Code_CODE_PERMISSION_DENIED: @@ -118,11 +119,12 @@ func (s *svc) handleDelete(ctx context.Context, w http.ResponseWriter, r *http.R return } if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we do not leak existence of a file + // return not found error so we dont leak existence of a file // TODO hide permission failed for users without access in every kind of request // TODO should this be done in the driver? status = http.StatusNotFound - m = "Resource not found" // mimic the oc10 error message + // TODO path might be empty or relative... + m = fmt.Sprintf("%s not fount", ref.Path) } w.WriteHeader(status) b, err := errors.Marshal(status, m, "") diff --git a/internal/http/services/owncloud/ocdav/locks.go b/internal/http/services/owncloud/ocdav/locks.go index f7dc30cfcc..99500580f0 100644 --- a/internal/http/services/owncloud/ocdav/locks.go +++ b/internal/http/services/owncloud/ocdav/locks.go @@ -400,12 +400,23 @@ func (s *svc) handleSpacesLock(w http.ResponseWriter, r *http.Request, spaceID s span.SetAttributes(attribute.String("component", "ocdav")) - ref, err := spacelookup.MakeStorageSpaceReference(spaceID, r.URL.Path) + client, err := s.getClient() if err != nil { - return http.StatusBadRequest, fmt.Errorf("invalid space id") + return http.StatusInternalServerError, err } - return s.lockReference(ctx, w, r, &ref) + // retrieve a specific storage space + space, cs3Status, err := spacelookup.LookUpStorageSpaceByID(ctx, client, spaceID) + if err != nil { + return http.StatusInternalServerError, err + } + if cs3Status.Code != rpc.Code_CODE_OK { + return http.StatusInternalServerError, errtypes.NewErrtypeFromStatus(cs3Status) + } + + ref := spacelookup.MakeRelativeReference(space, r.URL.Path, true) + + return s.lockReference(ctx, w, r, ref) } func (s *svc) lockReference(ctx context.Context, w http.ResponseWriter, r *http.Request, ref *provider.Reference) (retStatus int, retErr error) { diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index e6a318b4e9..6a16925e3e 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -136,10 +136,10 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re return http.StatusInternalServerError, err } if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we do not leak existence of a file + // return not found error so we dont leak existence of a file // TODO hide permission failed for users without access in every kind of request // TODO should this be done in the driver? - return http.StatusNotFound, fmt.Errorf("Resource not found") + return http.StatusNotFound, fmt.Errorf(sRes.Status.Message) } return http.StatusForbidden, fmt.Errorf(sRes.Status.Message) case res.Status.Code == rpc.Code_CODE_ABORTED: diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index 2010ecb123..e9be4a4ab0 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -36,7 +36,6 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/internal/grpc/services/storageprovider" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" @@ -46,18 +45,16 @@ import ( "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/publicshare" - rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/rhttp/router" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/rs/zerolog" - "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" - "google.golang.org/protobuf/types/known/fieldmaskpb" ) const ( - tracerName = "ocdav" + tracerName = "ocdav" + _spaceTypeProject = "project" ) type countingReader struct { @@ -202,7 +199,6 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns return } - // TODO look up all spaces and request the root_info in the field mask spaces, rpcStatus, err := spacelookup.LookUpStorageSpacesForPathWithChildren(ctx, client, fn) if err != nil { sublog.Error().Err(err).Msg("error sending a grpc request") @@ -215,12 +211,28 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns return } - resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, sublog) + var root *provider.StorageSpace + + switch { + case len(spaces) == 1: + root = spaces[0] + case len(spaces) > 1: + for _, space := range spaces { + if isVirtualRootResourceID(space.Root) { + root = space + } + } + if root == nil { + root = spaces[0] + } + } + + resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, false, sublog) if !ok { // getResourceInfos handles responses in case of an error so we can just return here. return } - p.propfindResponse(ctx, w, r, ns, pf, sendTusHeaders, resourceInfos, sublog) + p.propfindResponse(ctx, w, r, ns, root.SpaceType, pf, sendTusHeaders, resourceInfos, sublog) } // HandleSpacesPropfind handles a spaces based propfind request @@ -229,15 +241,10 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s defer span.End() sublog := appctx.GetLogger(ctx).With().Str("path", r.URL.Path).Str("spaceid", spaceID).Logger() - dh := r.Header.Get(net.HeaderDepth) - - depth, err := net.ParseDepth(dh) + client, err := p.getClient() if err != nil { - sublog.Debug().Str("depth", dh).Msg(err.Error()) - w.WriteHeader(http.StatusBadRequest) - m := fmt.Sprintf("Invalid Depth header value: %v", dh) - b, err := errors.Marshal(http.StatusBadRequest, m, "") - errors.HandleWebdavError(&sublog, w, b, err) + sublog.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) return } @@ -248,121 +255,35 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s return } - ref, err := spacelookup.MakeStorageSpaceReference(spaceID, r.URL.Path) - if err != nil { - sublog.Debug().Msg("invalid space id") - w.WriteHeader(http.StatusBadRequest) - m := fmt.Sprintf("Invalid space id: %v", spaceID) - b, err := errors.Marshal(http.StatusBadRequest, m, "") - errors.HandleWebdavError(&sublog, w, b, err) - return - } - - client, err := p.getClient() + // retrieve a specific storage space + space, rpcStatus, err := spacelookup.LookUpStorageSpaceByID(ctx, client, spaceID) if err != nil { - sublog.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error looking up the space by id") w.WriteHeader(http.StatusInternalServerError) return } - metadataKeys, _ := metadataKeys(pf) - - // stat the reference and request the space in the field mask - res, err := client.Stat(ctx, &provider.StatRequest{ - Ref: &ref, - ArbitraryMetadataKeys: metadataKeys, - FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"*"}}, // TODO use more sophisticated filter? we don't need all space properties, afaict only the spacetype - }) - if err != nil { - sublog.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return - } - if res.Status.Code != rpc.Code_CODE_OK { - status := rstatus.HTTPStatusFromCode(res.Status.Code) - if res.Status.Code == rpc.Code_CODE_ABORTED { - // aborted is used for etag an lock mismatches, which translates to 412 - // in case a real Conflict response is needed, the calling code needs to send the header - status = http.StatusPreconditionFailed - } - m := res.Status.Message - if res.Status.Code == rpc.Code_CODE_PERMISSION_DENIED { - // check if user has access to resource - sRes, err := client.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: ref.GetResourceId()}}) - if err != nil { - sublog.Error().Err(err).Msg("error performing stat grpc request") - w.WriteHeader(http.StatusInternalServerError) - return - } - if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we do not leak existence of a space - status = http.StatusNotFound - } - } - if status == http.StatusNotFound { - m = "Resource not found" // mimic the oc10 error message - } - w.WriteHeader(status) - b, err := errors.Marshal(status, m, "") - errors.HandleWebdavError(&sublog, w, b, err) + if rpcStatus.Code != rpc.Code_CODE_OK { + errors.HandleErrorStatus(&sublog, w, rpcStatus) return } - var space *provider.StorageSpace - if res.Info.Space == nil { - sublog.Debug().Msg("stat did not include a space, executing an additional lookup request") - // fake a space root - space = &provider.StorageSpace{ - Id: &provider.StorageSpaceId{OpaqueId: spaceID}, - Opaque: &typesv1beta1.Opaque{ - Map: map[string]*typesv1beta1.OpaqueEntry{ - "path": { - Decoder: "plain", - Value: []byte("/"), - }, - }, - }, - Root: ref.ResourceId, - RootInfo: res.Info, - } - } - res.Info.Path = r.URL.Path - - resourceInfos := []*provider.ResourceInfo{ - res.Info, - } - if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != net.DepthZero { - childInfos, ok := p.getSpaceResourceInfos(ctx, w, r, pf, &ref, r.URL.Path, depth, sublog) - if !ok { - // getResourceInfos handles responses in case of an error so we can just return here. - return - } - resourceInfos = append(resourceInfos, childInfos...) + resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, []*provider.StorageSpace{space}, r.URL.Path, true, sublog) + if !ok { + // getResourceInfos handles responses in case of an error so we can just return here. + return } // prefix space id to paths for i := range resourceInfos { resourceInfos[i].Path = path.Join("/", spaceID, resourceInfos[i].Path) - // add space to info so propfindResponse can access space type - if resourceInfos[i].Space == nil { - resourceInfos[i].Space = space - } - } - - sendTusHeaders := true - // let clients know this collection supports tus.io POST requests to start uploads - if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { - if res.Info.Opaque != nil { - _, ok := res.Info.Opaque.Map["disable_tus"] - sendTusHeaders = !ok - } } - p.propfindResponse(ctx, w, r, "", pf, sendTusHeaders, resourceInfos, sublog) + p.propfindResponse(ctx, w, r, "", space.SpaceType, pf, sendTusHeaders, resourceInfos, sublog) } -func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r *http.Request, namespace string, pf XML, sendTusHeaders bool, resourceInfos []*provider.ResourceInfo, log zerolog.Logger) { +func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r *http.Request, namespace, spaceType string, pf XML, sendTusHeaders bool, resourceInfos []*provider.ResourceInfo, log zerolog.Logger) { ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() @@ -381,27 +302,20 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r var linkshares map[string]struct{} // public link access does not show share-types - // oc:share-type is not part of an allprops response if namespace != "/public" { - // only fetch this if property was queried - for _, p := range pf.Prop { - if p.Space == net.NsOwncloud && (p.Local == "share-types" || p.Local == "permissions") { - listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) - if err == nil { - linkshares = make(map[string]struct{}, len(listResp.Share)) - for i := range listResp.Share { - linkshares[listResp.Share[i].ResourceId.OpaqueId] = struct{}{} - } - } else { - log.Error().Err(err).Msg("propfindResponse: couldn't list public shares") - span.SetStatus(codes.Error, err.Error()) - } - break + listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) + if err == nil { + linkshares = make(map[string]struct{}, len(listResp.Share)) + for i := range listResp.Share { + linkshares[listResp.Share[i].ResourceId.OpaqueId] = struct{}{} } + } else { + log.Error().Err(err).Msg("propfindResponse: couldn't list public shares") + span.SetStatus(codes.Error, err.Error()) } } - propRes, err := MultistatusResponse(ctx, &pf, resourceInfos, p.PublicURL, namespace, linkshares) + propRes, err := MultistatusResponse(ctx, &pf, resourceInfos, p.PublicURL, namespace, spaceType, linkshares) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) @@ -423,11 +337,10 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r } // TODO this is just a stat -> rename -func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference, metadataKeys, fieldMaskPaths []string) (*provider.ResourceInfo, *rpc.Status, error) { +func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient, space *provider.StorageSpace, ref *provider.Reference, metadataKeys []string) (*provider.ResourceInfo, *rpc.Status, error) { req := &provider.StatRequest{ Ref: ref, ArbitraryMetadataKeys: metadataKeys, - FieldMask: &fieldmaskpb.FieldMask{Paths: fieldMaskPaths}, } res, err := client.Stat(ctx, req) if err != nil { @@ -436,21 +349,17 @@ func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient return res.GetInfo(), res.GetStatus(), nil } -func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, spaces []*provider.StorageSpace, requestPath string, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) { - ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "get_resource_infos") - span.SetAttributes(attribute.KeyValue{Key: "requestPath", Value: attribute.StringValue(requestPath)}) - defer span.End() - +func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, spaces []*provider.StorageSpace, requestPath string, spacesPropfind bool, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) { dh := r.Header.Get(net.HeaderDepth) depth, err := net.ParseDepth(dh) if err != nil { + log.Debug().Str("depth", dh).Msg(err.Error()) w.WriteHeader(http.StatusBadRequest) m := fmt.Sprintf("Invalid Depth header value: %v", dh) b, err := errors.Marshal(http.StatusBadRequest, m, "") errors.HandleWebdavError(&log, w, b, err) return nil, false, false } - span.SetAttributes(attribute.KeyValue{Key: "depth", Value: attribute.StringValue(depth.String())}) client, err := p.getClient() if err != nil { @@ -459,7 +368,23 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r return nil, false, false } - metadataKeys, fieldMaskPaths := metadataKeys(pf) + var metadataKeys []string + + if pf.Allprop != nil { + // TODO this changes the behavior and returns all properties if allprops has been set, + // but allprops should only return some default properties + // see https://tools.ietf.org/html/rfc4918#section-9.1 + // the description of arbitrary_metadata_keys in https://cs3org.github.io/cs3apis/#cs3.storage.provider.v1beta1.ListContainerRequest an others may need clarification + // tracked in https://github.com/cs3org/cs3apis/issues/104 + metadataKeys = append(metadataKeys, "*") + } else { + metadataKeys = make([]string, 0, len(pf.Prop)) + for i := range pf.Prop { + if requiresExplicitFetching(&pf.Prop[i]) { + metadataKeys = append(metadataKeys, metadataKeyOf(&pf.Prop[i])) + } + } + } // we need to stat all spaces to aggregate the root etag, mtime and size // TODO cache per space (hah, no longer per user + per space!) @@ -470,41 +395,27 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r spaceMap = make(map[*provider.ResourceInfo]spaceData, len(spaces)) ) for _, space := range spaces { - spacePath := "" - if spacePath = utils.ReadPlainFromOpaque(space.Opaque, "path"); spacePath == "" { + spacePath, ok := getMountPoint(*space) + if !ok { continue // not mounted } - if space.RootInfo == nil { - spaceRef, err := spacelookup.MakeStorageSpaceReference(space.Id.OpaqueId, ".") - if err != nil { - continue - } - info, status, err := p.statSpace(ctx, client, &spaceRef, metadataKeys, fieldMaskPaths) - if err != nil || status.GetCode() != rpc.Code_CODE_OK { - continue - } - space.RootInfo = info - } - // TODO separate stats to the path or to the children, after statting all children update the mtime/etag // TODO get mtime, and size from space as well, so we no longer have to stat here? would require sending the requested metadata keys as well // root should be a ResourceInfo so it can contain the full stat, not only the id ... do we even need spaces then? // metadata keys could all be prefixed with "root." to indicate we want more than the root id ... - // TODO can we reuse the space.rootinfo? - spaceRef := spacelookup.MakeRelativeReference(space, requestPath, false) - var info *provider.ResourceInfo - if spaceRef.Path == "." && utils.ResourceIDEqual(spaceRef.ResourceId, space.Root) { - info = space.RootInfo - } else { - var status *rpc.Status - info, status, err = p.statSpace(ctx, client, spaceRef, metadataKeys, fieldMaskPaths) - if err != nil || status.GetCode() != rpc.Code_CODE_OK { - continue - } + spaceRef := spacelookup.MakeRelativeReference(space, requestPath, spacesPropfind) + info, status, err := p.statSpace(ctx, client, space, spaceRef, metadataKeys) + if err != nil || status.GetCode() != rpc.Code_CODE_OK { + continue } // adjust path - info.Path = filepath.Join(spacePath, spaceRef.Path) + if spacesPropfind { + // we need to prefix the path with / to make subsequent prefix matches work + info.Path = filepath.Join("/", spaceRef.Path) + } else { + info.Path = filepath.Join(spacePath, spaceRef.Path) + } spaceMap[info] = spaceData{Ref: spaceRef, SpaceType: space.SpaceType} @@ -559,12 +470,12 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r childInfos := map[string]*provider.ResourceInfo{} for spaceInfo, spaceData := range spaceMap { switch { - case spaceInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != net.DepthInfinity: + case !spacesPropfind && spaceInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != net.DepthInfinity: addChild(childInfos, spaceInfo, requestPath, rootInfo) case spaceInfo.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == net.DepthOne: switch { - case strings.HasPrefix(requestPath, spaceInfo.Path) && spaceData.SpaceType != "virtual": + case strings.HasPrefix(requestPath, spaceInfo.Path) && (spacesPropfind || spaceData.SpaceType != "virtual"): req := &provider.ListContainerRequest{ Ref: spaceData.Ref, ArbitraryMetadataKeys: metadataKeys, @@ -625,6 +536,9 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r for i := len(res.Infos) - 1; i >= 0; i-- { // add path to resource res.Infos[i].Path = filepath.Join(info.Path, res.Infos[i].Path) + if spacesPropfind { + res.Infos[i].Path = utils.MakeRelativePath(res.Infos[i].Path) + } if res.Infos[i].Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { stack = append(stack, res.Infos[i]) } @@ -656,125 +570,6 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r return resourceInfos, sendTusHeaders, true } -func (p *Handler) getSpaceResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, ref *provider.Reference, requestPath string, depth net.Depth, log zerolog.Logger) ([]*provider.ResourceInfo, bool) { - ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "get_space_resource_infos") - span.SetAttributes(attribute.KeyValue{Key: "requestPath", Value: attribute.StringValue(requestPath)}) - span.SetAttributes(attribute.KeyValue{Key: "depth", Value: attribute.StringValue(depth.String())}) - defer span.End() - - client, err := p.getClient() - if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return nil, false - } - - metadataKeys, _ := metadataKeys(pf) - - resourceInfos := []*provider.ResourceInfo{} - - req := &provider.ListContainerRequest{ - Ref: ref, - ArbitraryMetadataKeys: metadataKeys, - FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"*"}}, // TODO use more sophisticated filter - } - res, err := client.ListContainer(ctx, req) - if err != nil { - log.Error().Err(err).Msg("error sending list container grpc request") - w.WriteHeader(http.StatusInternalServerError) - return nil, false - } - - if res.Status.Code != rpc.Code_CODE_OK { - log.Debug().Interface("status", res.Status).Msg("List Container not ok, skipping") - return nil, false - } - for _, info := range res.Infos { - info.Path = path.Join(requestPath, info.Path) - } - resourceInfos = append(resourceInfos, res.Infos...) - - if depth == net.DepthInfinity { - // use a stack to explore sub-containers breadth-first - stack := resourceInfos - for len(stack) != 0 { - info := stack[0] - stack = stack[1:] - - if info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER /*|| space.SpaceType == "virtual"*/ { - continue - } - req := &provider.ListContainerRequest{ - Ref: &provider.Reference{ - ResourceId: info.Id, - Path: ".", - }, - ArbitraryMetadataKeys: metadataKeys, - } - res, err := client.ListContainer(ctx, req) // FIXME public link depth infinity -> "gateway: could not find provider: gateway: error calling ListStorageProviders: rpc error: code = PermissionDenied desc = auth: core access token is invalid" - if err != nil { - log.Error().Err(err).Interface("info", info).Msg("error sending list container grpc request") - w.WriteHeader(http.StatusInternalServerError) - return nil, false - } - if res.Status.Code != rpc.Code_CODE_OK { - log.Debug().Interface("status", res.Status).Msg("List Container not ok, skipping") - continue - } - - // check sub-containers in reverse order and add them to the stack - // the reversed order here will produce a more logical sorting of results - for i := len(res.Infos) - 1; i >= 0; i-- { - // add path to resource - res.Infos[i].Path = filepath.Join(info.Path, res.Infos[i].Path) - res.Infos[i].Path = utils.MakeRelativePath(res.Infos[i].Path) - if res.Infos[i].Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { - stack = append(stack, res.Infos[i]) - } - } - - resourceInfos = append(resourceInfos, res.Infos...) - // TODO: stream response to avoid storing too many results in memory - // we can do that after having stated the root. - } - } - - return resourceInfos, true -} - -// metadataKeys splits the propfind properties into arbitrary metadata and ResourceInfo field mask paths -func metadataKeys(pf XML) ([]string, []string) { - - var metadataKeys []string - var fieldMaskKeys []string - - if pf.Allprop != nil { - // TODO this changes the behavior and returns all properties if allprops has been set, - // but allprops should only return some default properties - // see https://tools.ietf.org/html/rfc4918#section-9.1 - // the description of arbitrary_metadata_keys in https://cs3org.github.io/cs3apis/#cs3.storage.provider.v1beta1.ListContainerRequest an others may need clarification - // tracked in https://github.com/cs3org/cs3apis/issues/104 - metadataKeys = append(metadataKeys, "*") - fieldMaskKeys = append(fieldMaskKeys, "*") - } else { - metadataKeys = make([]string, 0, len(pf.Prop)) - fieldMaskKeys = make([]string, 0, len(pf.Prop)) - for i := range pf.Prop { - if requiresExplicitFetching(&pf.Prop[i]) { - key := metadataKeyOf(&pf.Prop[i]) - switch key { - case "share-types": - fieldMaskKeys = append(fieldMaskKeys, key) - default: - metadataKeys = append(metadataKeys, key) - } - - } - } - } - return metadataKeys, fieldMaskKeys -} - func addChild(childInfos map[string]*provider.ResourceInfo, spaceInfo *provider.ResourceInfo, requestPath string, @@ -809,6 +604,15 @@ func addChild(childInfos map[string]*provider.ResourceInfo, } } +func getMountPoint(space provider.StorageSpace) (string, bool) { + if space.Opaque == nil || + space.Opaque.Map["path"] == nil || + space.Opaque.Map["path"].Decoder != "plain" { + return "", false + } + return string(space.Opaque.Map["path"].Value), true +} + func requiresExplicitFetching(n *xml.Name) bool { switch n.Space { case net.NsDav: @@ -866,10 +670,10 @@ func ReadPropfind(r io.Reader) (pf XML, status int, err error) { } // MultistatusResponse converts a list of resource infos into a multistatus response string -func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) ([]byte, error) { +func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns, spaceType string, linkshares map[string]struct{}) ([]byte, error) { responses := make([]*ResponseXML, 0, len(mds)) for i := range mds { - res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, linkshares) + res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, spaceType, linkshares) if err != nil { return nil, err } @@ -888,12 +692,7 @@ func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceI // mdToPropResponse converts the CS3 metadata into a webdav PropResponse // ns is the CS3 namespace that needs to be removed from the CS3 path before // prefixing it with the baseURI -func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, publicURL, ns string, linkshares map[string]struct{}) (*ResponseXML, error) { - ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "md_to_prop_response") - span.SetAttributes(attribute.KeyValue{Key: "publicURL", Value: attribute.StringValue(publicURL)}) - span.SetAttributes(attribute.KeyValue{Key: "ns", Value: attribute.StringValue(ns)}) - defer span.End() - +func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, publicURL, ns, spaceType string, linkshares map[string]struct{}) (*ResponseXML, error) { sublog := appctx.GetLogger(ctx).With().Interface("md", md).Str("ns", ns).Logger() md.Path = strings.TrimPrefix(md.Path, ns) @@ -917,7 +716,6 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p quota := net.PropQuotaUnknown size := strconv.FormatUint(md.Size, 10) var lock *provider.Lock - shareTypes := "" // TODO refactor helper functions: GetOpaqueJSONEncoded(opaque, key string, *struct) err, GetOpaquePlainEncoded(opaque, key) value, err // or use ok like pattern and return bool? if md.Opaque != nil && md.Opaque.Map != nil { @@ -928,8 +726,8 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p sublog.Error().Err(err).Msg("could not unmarshal link json") } } - if quota = utils.ReadPlainFromOpaque(md.Opaque, "quota"); quota == "" { - quota = net.PropQuotaUnknown + if md.Opaque.Map["quota"] != nil && md.Opaque.Map["quota"].Decoder == "plain" { + quota = string(md.Opaque.Map["quota"].Value) } if md.Opaque.Map["lock"] != nil && md.Opaque.Map["lock"].Decoder == "json" { lock = &provider.Lock{} @@ -938,18 +736,13 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p sublog.Error().Err(err).Msg("could not unmarshal locks json") } } - shareTypes = utils.ReadPlainFromOpaque(md.Opaque, "share-types") } role := conversions.RoleFromResourcePermissions(md.PermissionSet) - if md.Space != nil && md.Space.SpaceType != "grant" && utils.ResourceIDEqual(md.Space.Root, md.Id) { - // a space root is never shared - shareTypes = "" - } + isShared := spaceType != _spaceTypeProject && !net.IsCurrentUserOwner(ctx, md.Owner) var wdp string isPublic := ls != nil - isShared := shareTypes != "" && !net.IsCurrentUserOwner(ctx, md.Owner) if md.PermissionSet != nil { wdp = role.WebDAVPermissions( md.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER, @@ -957,6 +750,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p false, isPublic, ) + sublog.Debug().Interface("role", role).Str("dav-permissions", wdp).Msg("converted PermissionSet") } // replace fileid of /public/{token} mountpoint with grant fileid @@ -1232,19 +1026,14 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { propstatNotFound.Prop = append(propstatNotFound.Prop, prop.NotFound("oc:checksums")) } - case "share-types": // used to render share indicators to share owners + case "share-types": // desktop var types strings.Builder - - sts := strings.Split(shareTypes, ",") - for _, shareType := range sts { - switch shareType { - case "1": // provider.GranteeType_GRANTEE_TYPE_USER - types.WriteString("" + strconv.Itoa(int(conversions.ShareTypeUser)) + "") - case "2": // provider.GranteeType_GRANTEE_TYPE_GROUP - types.WriteString("" + strconv.Itoa(int(conversions.ShareTypeGroup)) + "") - default: - sublog.Debug().Interface("shareType", shareType).Msg("unknown share type, ignoring") - } + k := md.GetArbitraryMetadata() + amd := k.GetMetadata() + if amdv, ok := amd[metadataKeyOf(&pf.Prop[i])]; ok { + types.WriteString("") + types.WriteString(amdv) + types.WriteString("") } if md.Id != nil { @@ -1533,17 +1322,12 @@ func (c *countingReader) Read(p []byte) (int, error) { } func metadataKeyOf(n *xml.Name) string { - switch n.Space { - case net.NsDav: - if n.Local == "quota-available-bytes" { - return "quota" - } - case net.NsOwncloud: - if n.Local == "share-types" { - return "share-types" - } + switch { + case n.Space == net.NsDav && n.Local == "quota-available-bytes": + return "quota" + default: + return fmt.Sprintf("%s/%s", n.Space, n.Local) } - return fmt.Sprintf("%s/%s", n.Space, n.Local) } // UnmarshalXML appends the property names enclosed within start to pn. @@ -1577,3 +1361,15 @@ func (pn *Props) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { } } } + +// isVirtualRootResourceID returns true if the id points to the share jail root. The providerid is optional for legacy ids +func isVirtualRootResourceID(id *provider.ResourceId) bool { + switch { + case id == nil: + return false + case id.OpaqueId != utils.ShareStorageProviderID: + return false + } + providerID, spaceID := storagespace.SplitStorageID(id.StorageId) + return spaceID == utils.ShareStorageProviderID && (providerID == "" || providerID == utils.ShareStorageProviderID) +} diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go index 0816ed7ef6..daafb791e6 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go @@ -34,7 +34,6 @@ import ( "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/rgrpc/status" - "github.com/cs3org/reva/v2/pkg/utils" "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -65,7 +64,7 @@ var _ = Describe("Propfind", func() { mockStat = func(ref *sprovider.Reference, info *sprovider.ResourceInfo) { client.On("Stat", mock.Anything, mock.MatchedBy(func(req *sprovider.StatRequest) bool { - return utils.ResourceIDEqual(req.Ref.ResourceId, ref.ResourceId) && + return (ref.ResourceId.GetOpaqueId() == "" || req.Ref.ResourceId.GetOpaqueId() == ref.ResourceId.GetOpaqueId()) && (ref.Path == "" || req.Ref.Path == ref.Path) })).Return(&sprovider.StatResponse{ Status: status.NewOK(ctx), @@ -74,7 +73,7 @@ var _ = Describe("Propfind", func() { } mockListContainer = func(ref *sprovider.Reference, infos []*sprovider.ResourceInfo) { client.On("ListContainer", mock.Anything, mock.MatchedBy(func(req *sprovider.ListContainerRequest) bool { - match := utils.ResourceIDEqual(req.Ref.ResourceId, ref.ResourceId) && + match := (ref.ResourceId.GetOpaqueId() == "" || req.Ref.ResourceId.GetOpaqueId() == ref.ResourceId.GetOpaqueId()) && (ref.Path == "" || req.Ref.Path == ref.Path) return match })).Return(&sprovider.ListContainerResponse{ @@ -83,20 +82,6 @@ var _ = Describe("Propfind", func() { }, nil) } - foospace *sprovider.StorageSpace - fooquxspace *sprovider.StorageSpace - fooFileShareSpace *sprovider.StorageSpace - fooFileShare2Space *sprovider.StorageSpace - fooDirShareSpace *sprovider.StorageSpace - ) - - JustBeforeEach(func() { - ctx = context.WithValue(context.Background(), net.CtxKeyBaseURI, "http://127.0.0.1:3000") - client = &mocks.GatewayAPIClient{} - handler = propfind.NewHandler("127.0.0.1:3000", func() (gateway.GatewayAPIClient, error) { - return client, nil - }) - foospace = &sprovider.StorageSpace{ Opaque: &typesv1beta1.Opaque{ Map: map[string]*typesv1beta1.OpaqueEntry{ @@ -106,8 +91,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "foospace!root"}, - Root: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "foospace"}, + Root: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Name: "foospace", } fooquxspace = &sprovider.StorageSpace{ @@ -119,8 +104,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooquxspace!root"}, - Root: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooquxspace"}, + Root: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot"}, Name: "fooquxspace", } fooFileShareSpace = &sprovider.StorageSpace{ @@ -132,8 +117,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace!sharedfile"}, - Root: &sprovider.ResourceId{StorageId: "fooFileShareSpace", OpaqueId: "sharedfile"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace"}, + Root: &sprovider.ResourceId{OpaqueId: "sharedfile"}, Name: "fooFileShareSpace", } fooFileShare2Space = &sprovider.StorageSpace{ @@ -145,8 +130,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace2!sharedfile2"}, - Root: &sprovider.ResourceId{StorageId: "fooFileShareSpace2", OpaqueId: "sharedfile2"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace2"}, + Root: &sprovider.ResourceId{OpaqueId: "sharedfile2"}, Name: "fooFileShareSpace2", } fooDirShareSpace = &sprovider.StorageSpace{ @@ -158,131 +143,80 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooDirShareSpace!shareddir"}, - Root: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooDirShareSpace"}, + Root: &sprovider.ResourceId{OpaqueId: "shareddir"}, Name: "fooDirShareSpace", } + ) - // For the space mounted a /foo we assign a storageid "foospace" and a root opaqueid "root" - // it contains four resources - // - ./bar, file, 100 bytes, opaqueid "bar" - // - ./baz, file, 1 byte, opaqueid "baz" - // - ./dir, folder, 30 bytes, opaqueid "dir" - // - ./dir/entry, file, 30 bytes, opaqueid "direntry" - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "."}, + JustBeforeEach(func() { + ctx = context.WithValue(context.Background(), net.CtxKeyBaseURI, "http://127.0.0.1:3000") + client = &mocks.GatewayAPIClient{} + handler = propfind.NewHandler("127.0.0.1:3000", func() (gateway.GatewayAPIClient, error) { + return client, nil + }) + + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, + Id: &sprovider.ResourceId{OpaqueId: "foospaceroot", StorageId: "foospaceroot"}, Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, Path: ".", Size: uint64(131), }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "."}, []*sprovider.ResourceInfo{ { Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, Path: "bar", Size: 100, }, { Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "baz"}, Path: "baz", Size: 1, }, { Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dir"}, Path: "dir", Size: 30, }, }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "./bar"}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "./bar", - Size: uint64(100), - }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, Path: "."}, + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "./bar"}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, + Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "foospacebar"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "./bar", Size: uint64(100), }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "./baz"}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "baz"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "./baz", - Size: uint64(1), - }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "baz"}, Path: "."}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "baz"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "./baz", - Size: uint64(1), - }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "./dir"}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dir"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, - Path: "./dir", - Size: uint64(30), - }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dir"}, Path: "."}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dir"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, - Path: "./dir", - Size: uint64(30), - }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Path: "./dir"}, - []*sprovider.ResourceInfo{ - { - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "direntry"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "entry", - Size: 30, - }, - }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dir"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "./dir"}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "direntry"}, + Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dirent"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "entry", Size: 30, }, }) - // For the space mounted a /foo/qux we assign a storageid "foospace" and a root opaqueid "root" - // it contains one resource - // - ./quux, file, 1000 bytes, opaqueid "quux" - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, Path: "."}, + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot"}, Path: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, Path: ".", Size: uint64(1000), }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot"}, Path: "."}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "quux"}, + Id: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot", StorageId: "fooquxspaceroot"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "./quux", + Path: "quux", Size: 1000, }, }) - // For the space mounted a /foo/Shares/sharedFile we assign a storageid "fooFileShareSpace" and a root opaqueid "sharedfile" - // it is a file resource, 2000 bytes, opaqueid "sharedfile" - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooFileShareSpace", OpaqueId: "sharedfile"}, Path: "."}, + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "sharedfile"}, Path: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "fooFileShareSpace", OpaqueId: "sharedfile"}, + Id: &sprovider.ResourceId{OpaqueId: "sharedfile", StorageId: "sharedfile"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: ".", Size: uint64(2000), @@ -290,34 +224,28 @@ var _ = Describe("Propfind", func() { Etag: "1", }) - // For the space mounted a /foo/Shares/sharedFile2 we assign a storageid "fooFileShareSpace2" and a root opaqueid "sharedfile2" - // it is a file resource, 2500 bytes, opaqueid "sharedfile2" - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooFileShareSpace2", OpaqueId: "sharedfile2"}, Path: "."}, + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "sharedfile2"}, Path: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "fooFileShareSpace2", OpaqueId: "sharedfile2"}, + Id: &sprovider.ResourceId{OpaqueId: "sharedfile2", StorageId: "sharedfile2"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: ".", Size: uint64(2500), Mtime: &typesv1beta1.Timestamp{Seconds: 2}, Etag: "2", }) - - // For the space mounted a /foo/Shares/sharedFile2 we assign a storageid "fooDirShareSpace" and a root opaqueid "shareddir" - // it is a folder containing one resource - // ./something, file, 1500 bytes, opaqueid "shareddirsomething" - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, Path: "."}, + mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "shareddir"}, Path: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, + Id: &sprovider.ResourceId{OpaqueId: "shareddir", StorageId: "shareddir"}, Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, Path: ".", Size: uint64(1500), Mtime: &typesv1beta1.Timestamp{Seconds: 3}, Etag: "3", }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "shareddir"}, Path: "."}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddirsomething"}, + Id: &sprovider.ResourceId{OpaqueId: "shareddir", StorageId: "shareddir"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "something", Size: 1500, @@ -333,11 +261,11 @@ var _ = Describe("Propfind", func() { } else { term := req.Filters[0].Term.(*link.ListPublicSharesRequest_Filter_ResourceId) switch { - case term != nil && term.ResourceId != nil && term.ResourceId.OpaqueId == "bar": + case term != nil && term.ResourceId != nil && term.ResourceId.OpaqueId == "foospacebar": shares = []*link.PublicShare{ { Id: &link.PublicShareId{OpaqueId: "share1"}, - ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, + ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "foospacebar"}, }, } default: @@ -371,13 +299,6 @@ var _ = Describe("Propfind", func() { Status: status.NewOK(ctx), StorageSpaces: []*sprovider.StorageSpace{}, }, nil) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospace", StorageId: "foospace"}, Path: "."}, - &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{OpaqueId: "foospace", StorageId: "foospace"}, - Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, - Path: ".", - Size: uint64(131), - }) }) It("verifies the depth header", func() { @@ -688,32 +609,27 @@ var _ = Describe("Propfind", func() { }) Describe("HandleSpacesPropfind", func() { - /* - JustBeforeEach(func() { - client.On("Stat", mock.Anything, mock.Anything).Return(func(_ context.Context, req *sprovider.StatRequest, _ ...grpc.CallOption) *sprovider.StatResponse { + JustBeforeEach(func() { + client.On("ListStorageSpaces", mock.Anything, mock.Anything).Return( + func(_ context.Context, req *sprovider.ListStorageSpacesRequest, _ ...grpc.CallOption) *sprovider.ListStorageSpacesResponse { + var spaces []*sprovider.StorageSpace switch { - case req.Ref.ResourceId.OpaqueId == "foospace": - return &sprovider.StatResponse{ - Status: status.NewOK(ctx), - Info: &sprovider.ResourceInfo{ - Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, - Id: &sprovider.ResourceId{OpaqueId: "foospaceroot", StorageId: "foospaceroot"}, - Size: 131, - Path: ".", - }, - } + case req.Filters[0].Term.(*sprovider.ListStorageSpacesRequest_Filter_Id).Id.OpaqueId == "foospace": + spaces = []*sprovider.StorageSpace{foospace} default: - return &sprovider.StatResponse{ - Status: status.NewNotFound(ctx, "not found"), - } + spaces = []*sprovider.StorageSpace{} + } + return &sprovider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: spaces, } }, nil) - }) - */ + }) It("handles invalid space ids", func() { - client.On("Stat", mock.Anything, mock.Anything).Return(&sprovider.StatResponse{ - Status: status.NewNotFound(ctx, "not found"), + client.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&sprovider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*sprovider.StorageSpace{}, }, nil) rr := httptest.NewRecorder() @@ -725,15 +641,12 @@ var _ = Describe("Propfind", func() { }) It("stats the space root", func() { - client.On("Stat", mock.Anything, mock.Anything).Return(&sprovider.StatResponse{ - Status: status.NewNotFound(ctx, "not found"), - }, nil) rr := httptest.NewRecorder() req, err := http.NewRequest("GET", "/", strings.NewReader("")) Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace!root") + handler.HandleSpacesPropfind(rr, req, "foospace") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -741,19 +654,19 @@ var _ = Describe("Propfind", func() { Expect(len(res.Responses)).To(Equal(4)) root := res.Responses[0] - Expect(root.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/")) + Expect(root.Href).To(Equal("http:/127.0.0.1:3000/foospace/")) Expect(string(root.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("131")) bar := res.Responses[1] - Expect(bar.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/bar")) + Expect(bar.Href).To(Equal("http:/127.0.0.1:3000/foospace/bar")) Expect(string(bar.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("100")) baz := res.Responses[2] - Expect(baz.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/baz")) + Expect(baz.Href).To(Equal("http:/127.0.0.1:3000/foospace/baz")) Expect(string(baz.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("1")) dir := res.Responses[3] - Expect(dir.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/dir/")) + Expect(dir.Href).To(Equal("http:/127.0.0.1:3000/foospace/dir/")) Expect(string(dir.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) }) @@ -763,7 +676,7 @@ var _ = Describe("Propfind", func() { Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace!root") + handler.HandleSpacesPropfind(rr, req, "foospace") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -773,19 +686,30 @@ var _ = Describe("Propfind", func() { }) It("stats a directory", func() { + mockStat(&sprovider.Reference{Path: "./baz"}, &sprovider.ResourceInfo{ + Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, + Size: 50, + }) + mockListContainer(&sprovider.Reference{Path: "./baz"}, []*sprovider.ResourceInfo{ + { + Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, + Size: 50, + }, + }) + rr := httptest.NewRecorder() - req, err := http.NewRequest("GET", "/dir", strings.NewReader("")) + req, err := http.NewRequest("GET", "/baz", strings.NewReader("")) Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace!root") + handler.HandleSpacesPropfind(rr, req, "foospace") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) Expect(err).ToNot(HaveOccurred()) Expect(len(res.Responses)).To(Equal(2)) - Expect(string(res.Responses[0].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) - Expect(string(res.Responses[1].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) + Expect(string(res.Responses[0].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("50")) + Expect(string(res.Responses[1].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("50")) }) It("includes all the things™ when depth is infinity", func() { @@ -795,7 +719,7 @@ var _ = Describe("Propfind", func() { Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace!root") + handler.HandleSpacesPropfind(rr, req, "foospace") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -807,11 +731,11 @@ var _ = Describe("Propfind", func() { paths = append(paths, r.Href) } Expect(paths).To(ConsistOf( - "http:/127.0.0.1:3000/foospace%21root/", - "http:/127.0.0.1:3000/foospace%21root/bar", - "http:/127.0.0.1:3000/foospace%21root/baz", - "http:/127.0.0.1:3000/foospace%21root/dir/", - "http:/127.0.0.1:3000/foospace%21root/dir/entry", + "http:/127.0.0.1:3000/foospace/", + "http:/127.0.0.1:3000/foospace/bar", + "http:/127.0.0.1:3000/foospace/baz", + "http:/127.0.0.1:3000/foospace/dir/", + "http:/127.0.0.1:3000/foospace/dir/entry", )) }) }) diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 0714bf0a48..b4c9e948f3 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -180,13 +180,6 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt } if res.Status.Code != rpc.Code_CODE_OK { - status := rstatus.HTTPStatusFromCode(res.Status.Code) - if res.Status.Code == rpc.Code_CODE_ABORTED { - // aborted is used for etag an lock mismatches, which translates to 412 - // in case a real Conflict response is needed, the calling code needs to send the header - status = http.StatusPreconditionFailed - } - m := res.Status.Message if res.Status.Code == rpc.Code_CODE_PERMISSION_DENIED { // check if user has access to resource sRes, err := c.Stat(ctx, &provider.StatRequest{Ref: ref}) @@ -195,19 +188,21 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt w.WriteHeader(http.StatusInternalServerError) return nil, nil, false } + status := http.StatusForbidden + m := fmt.Sprintf("Permission denied to remove properties on resource %v", ref.Path) if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we do not leak existence of a file + // return not found error so we dont leak existence of a file // TODO hide permission failed for users without access in every kind of request // TODO should this be done in the driver? status = http.StatusNotFound + m = fmt.Sprintf("%v not found", ref.Path) } + w.WriteHeader(status) + b, err := errors.Marshal(status, m, "") + errors.HandleWebdavError(&log, w, b, err) + return nil, nil, false } - if status == http.StatusNotFound { - m = "Resource not found" // mimic the oc10 error message - } - w.WriteHeader(status) - b, err := errors.Marshal(status, m, "") - errors.HandleWebdavError(&log, w, b, err) + errors.HandleErrorStatus(&log, w, res.Status) return nil, nil, false } if key == "http://owncloud.org/ns/favorite" { @@ -234,13 +229,6 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt } if res.Status.Code != rpc.Code_CODE_OK { - status := rstatus.HTTPStatusFromCode(res.Status.Code) - if res.Status.Code == rpc.Code_CODE_ABORTED { - // aborted is used for etag an lock mismatches, which translates to 412 - // in case a real Conflict response is needed, the calling code needs to send the header - status = http.StatusPreconditionFailed - } - m := res.Status.Message if res.Status.Code == rpc.Code_CODE_PERMISSION_DENIED { // check if user has access to resource sRes, err := c.Stat(ctx, &provider.StatRequest{Ref: ref}) @@ -249,19 +237,21 @@ func (s *svc) handleProppatch(ctx context.Context, w http.ResponseWriter, r *htt w.WriteHeader(http.StatusInternalServerError) return nil, nil, false } + status := http.StatusForbidden + m := fmt.Sprintf("Permission denied to set properties on resource %v", ref.Path) if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we don't leak existence of a file + // return not found error so we dont leak existence of a file // TODO hide permission failed for users without access in every kind of request // TODO should this be done in the driver? status = http.StatusNotFound + m = fmt.Sprintf("%v not found", ref.Path) } + w.WriteHeader(status) + b, err := errors.Marshal(status, m, "") + errors.HandleWebdavError(&log, w, b, err) + return nil, nil, false } - if status == http.StatusNotFound { - m = "Resource not found" // mimic the oc10 error message - } - w.WriteHeader(status) - b, err := errors.Marshal(status, m, "") - errors.HandleWebdavError(&log, w, b, err) + errors.HandleErrorStatus(&log, w, res.Status) return nil, nil, false } diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index da8a341ed4..fc19def345 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -111,7 +111,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s infos := s.getPublicFileInfos(onContainer, depth == net.DepthZero, tokenStatInfo) - propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, ns, nil) + propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, ns, "", nil) if err != nil { sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index fe7fab892b..568f8cd93b 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -20,6 +20,7 @@ package ocdav import ( "context" + "fmt" "net/http" "path" "strconv" @@ -232,13 +233,11 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ return } if sRes.Status.Code != rpc.Code_CODE_OK { - // return not found error so we do not leak existence of a file + // return not found error so we dont leak existence of a file // TODO hide permission failed for users without access in every kind of request // TODO should this be done in the driver? status = http.StatusNotFound - } - if status == http.StatusNotFound { - m = "Resource not found" // mimic the oc10 error message + m = fmt.Sprintf("%v not found", ref.Path) } w.WriteHeader(status) b, err := errors.Marshal(status, m, "") diff --git a/internal/http/services/owncloud/ocdav/report.go b/internal/http/services/owncloud/ocdav/report.go index b5ad80a537..1c93756072 100644 --- a/internal/http/services/owncloud/ocdav/report.go +++ b/internal/http/services/owncloud/ocdav/report.go @@ -110,7 +110,7 @@ func (s *svc) doFilterFiles(w http.ResponseWriter, r *http.Request, ff *reportFi infos = append(infos, statRes.Info) } - responsesXML, err := propfind.MultistatusResponse(ctx, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, nil) + responsesXML, err := propfind.MultistatusResponse(ctx, &propfind.XML{Prop: ff.Prop}, infos, s.c.PublicURL, namespace, "", nil) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) diff --git a/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go b/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go index a7b4afe132..c7cbc7eb78 100644 --- a/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go +++ b/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go @@ -31,7 +31,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" - "google.golang.org/protobuf/types/known/fieldmaskpb" ) // LookupReferenceForPath returns: @@ -99,13 +98,12 @@ func LookUpStorageSpacesForPathWithChildren(ctx context.Context, client gateway. // TODO use ListContainerStream to listen for changes // retrieve a specific storage space lSSReq := &storageProvider.ListStorageSpacesRequest{ - // get all fields, including root_info - FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"*"}}, + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + "path": {Decoder: "plain", Value: []byte(path)}, + "withChildMounts": {Decoder: "plain", Value: []byte("true")}, + }}, } - // list all providers at or below the given path - lSSReq.Opaque = utils.AppendPlainToOpaque(lSSReq.Opaque, "path", path) - // we want to get all metadata? really? when looking up the space roots we actually only want etag, mtime and type so we can construct a child ... - lSSReq.Opaque = utils.AppendPlainToOpaque(lSSReq.Opaque, "metadata", "*") lSSRes, err := client.ListStorageSpaces(ctx, lSSReq) if err != nil { diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index 24e4696ff9..37b2d935d4 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -189,7 +189,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, infos = append(infos, vi) } - propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, "", nil) + propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, "", "", nil) if err != nil { sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go index 3e46f65104..1813a8e732 100644 --- a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go +++ b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go @@ -88,8 +88,8 @@ func New(m map[string]interface{}) (storage.FS, error) { // We need to override the two methods, GetMD and ListFolder to fill the // StorageId in the ResourceInfo objects. -func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { - res, err := w.FS.GetMD(ctx, ref, mdKeys, fieldMask) +func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { + res, err := w.FS.GetMD(ctx, ref, mdKeys) if err != nil { return nil, err } @@ -102,8 +102,8 @@ func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []s return res, nil } -func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { - res, err := w.FS.ListFolder(ctx, ref, mdKeys, fieldMask) +func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { + res, err := w.FS.ListFolder(ctx, ref, mdKeys) if err != nil { return nil, err } diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index 3c7cad877f..bf247a1c28 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -107,8 +107,8 @@ func New(m map[string]interface{}) (storage.FS, error) { // We need to override the methods, GetMD, GetPathByID and ListFolder to fill the // StorageId in the ResourceInfo objects. -func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { - res, err := w.FS.GetMD(ctx, ref, mdKeys, fieldMask) +func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { + res, err := w.FS.GetMD(ctx, ref, mdKeys) if err != nil { return nil, err } @@ -132,8 +132,8 @@ func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []s return res, nil } -func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { - res, err := w.FS.ListFolder(ctx, ref, mdKeys, fieldMask) +func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { + res, err := w.FS.ListFolder(ctx, ref, mdKeys) if err != nil { return nil, err } @@ -272,7 +272,7 @@ func (w *wrapper) userIsProjectAdmin(ctx context.Context, ref *provider.Referenc return nil } - res, err := w.FS.GetMD(ctx, ref, nil, nil) + res, err := w.FS.GetMD(ctx, ref, nil) if err != nil { return err } diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go index 77f6960cc5..356545bab9 100644 --- a/pkg/rhttp/datatx/utils/download/download.go +++ b/pkg/rhttp/datatx/utils/download/download.go @@ -69,7 +69,7 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI // do a stat to set a Content-Length header - if md, err = fs.GetMD(ctx, ref, nil, []string{"size", "mimetype"}); err != nil { + if md, err = fs.GetMD(ctx, ref, nil); err != nil { handleError(w, &sublog, err, "stat") return } diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index bd75318d5f..94cf4be949 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -301,8 +301,7 @@ func (nc *StorageDriver) Move(ctx context.Context, oldRef, newRef *provider.Refe } // GetMD as defined in the storage.FS interface -// TODO forward fieldMask -func (nc *StorageDriver) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { +func (nc *StorageDriver) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { type paramsObj struct { Ref *provider.Reference `json:"ref"` MdKeys []string `json:"mdKeys"` @@ -332,7 +331,7 @@ func (nc *StorageDriver) GetMD(ctx context.Context, ref *provider.Reference, mdK } // ListFolder as defined in the storage.FS interface -func (nc *StorageDriver) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { +func (nc *StorageDriver) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { type paramsObj struct { Ref *provider.Reference `json:"ref"` MdKeys []string `json:"mdKeys"` diff --git a/pkg/storage/fs/nextcloud/nextcloud_test.go b/pkg/storage/fs/nextcloud/nextcloud_test.go index d74dc693ff..4852eb3122 100644 --- a/pkg/storage/fs/nextcloud/nextcloud_test.go +++ b/pkg/storage/fs/nextcloud/nextcloud_test.go @@ -224,7 +224,7 @@ var _ = Describe("Nextcloud", func() { Path: "/some/path", } mdKeys := []string{"val1", "val2", "val3"} - result, err := nc.GetMD(ctx, ref, mdKeys, nil) + result, err := nc.GetMD(ctx, ref, mdKeys) Expect(err).ToNot(HaveOccurred()) Expect(*result).To(Equal(provider.ResourceInfo{ Opaque: &types.Opaque{ @@ -319,7 +319,7 @@ var _ = Describe("Nextcloud", func() { Path: "/some", } mdKeys := []string{"val1", "val2", "val3"} - results, err := nc.ListFolder(ctx, ref, mdKeys, []string{}) + results, err := nc.ListFolder(ctx, ref, mdKeys) Expect(err).NotTo(HaveOccurred()) Expect(len(results)).To(Equal(1)) Expect(*results[0]).To(Equal(provider.ResourceInfo{ diff --git a/pkg/storage/fs/owncloudsql/owncloudsql.go b/pkg/storage/fs/owncloudsql/owncloudsql.go index 41e0010d27..86d5e5deef 100644 --- a/pkg/storage/fs/owncloudsql/owncloudsql.go +++ b/pkg/storage/fs/owncloudsql/owncloudsql.go @@ -1307,7 +1307,7 @@ func (fs *owncloudsqlfs) Move(ctx context.Context, oldRef, newRef *provider.Refe return nil } -func (fs *owncloudsqlfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { +func (fs *owncloudsqlfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { ip, err := fs.resolve(ctx, ref) if err != nil { // TODO return correct errtype @@ -1351,7 +1351,7 @@ func (fs *owncloudsqlfs) GetMD(ctx context.Context, ref *provider.Reference, mdK return fs.convertToResourceInfo(ctx, entry, ip, mdKeys) } -func (fs *owncloudsqlfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { +func (fs *owncloudsqlfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { log := appctx.GetLogger(ctx) ip, err := fs.resolve(ctx, ref) diff --git a/pkg/storage/fs/s3/s3.go b/pkg/storage/fs/s3/s3.go index 9459f3751c..a174003bea 100644 --- a/pkg/storage/fs/s3/s3.go +++ b/pkg/storage/fs/s3/s3.go @@ -516,7 +516,7 @@ func (fs *s3FS) Move(ctx context.Context, oldRef, newRef *provider.Reference) er return nil } -func (fs *s3FS) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { +func (fs *s3FS) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { log := appctx.GetLogger(ctx) fn, err := fs.resolve(ctx, ref) @@ -579,7 +579,7 @@ func (fs *s3FS) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []str return fs.normalizeHead(ctx, output, fn), nil } -func (fs *s3FS) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { +func (fs *s3FS) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { fn, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "error resolving ref") diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index d02b912b5f..98068b715c 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -39,8 +39,8 @@ type FS interface { TouchFile(ctx context.Context, ref *provider.Reference) error Delete(ctx context.Context, ref *provider.Reference) error Move(ctx context.Context, oldRef, newRef *provider.Reference) error - GetMD(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) (*provider.ResourceInfo, error) - ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) + GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) + ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) InitiateUpload(ctx context.Context, ref *provider.Reference, uploadLength int64, metadata map[string]string) (map[string]string, error) Upload(ctx context.Context, ref *provider.Reference, r io.ReadCloser, uploadFunc UploadFinishedFunc) error Download(ctx context.Context, ref *provider.Reference) (io.ReadCloser, error) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 39a8694432..f4ec758038 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -173,8 +173,7 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) ( return 0, 0, 0, errtypes.PermissionDenied(n.ID) } - // FIXME move treesize & quota to fieldmask - ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, []string{}, true) + ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, true) if err != nil { return 0, 0, 0, err } @@ -292,7 +291,6 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference) if n, err = fs.lu.NodeFromResource(ctx, parentRef); err != nil { return } - // TODO check if user has access to root / space if !n.Exists { return errtypes.PreconditionFailed(parentRef.Path) } @@ -500,7 +498,7 @@ func (fs *Decomposedfs) Move(ctx context.Context, oldRef, newRef *provider.Refer } // GetMD returns the metadata for the specified resource -func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (ri *provider.ResourceInfo, err error) { +func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (ri *provider.ResourceInfo, err error) { var node *node.Node if node, err = fs.lu.NodeFromResource(ctx, ref); err != nil { return @@ -519,29 +517,11 @@ func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKe return nil, errtypes.PermissionDenied(node.ID) } - md, err := node.AsResourceInfo(ctx, &rp, mdKeys, fieldMask, utils.IsRelativeReference(ref)) - if err != nil { - return nil, err - } - - addSpace := len(fieldMask) == 0 - for _, p := range fieldMask { - if p == "space" || p == "*" { - addSpace = true - break - } - } - if addSpace { - if md.Space, err = fs.storageSpaceFromNode(ctx, node, node.InternalPath(), false, false); err != nil { - return nil, err - } - } - - return md, nil + return node.AsResourceInfo(ctx, &rp, mdKeys, utils.IsRelativeReference(ref)) } // ListFolder returns a list of resources in the specified folder -func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (finfos []*provider.ResourceInfo, err error) { +func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) (finfos []*provider.ResourceInfo, err error) { var n *node.Node if n, err = fs.lu.NodeFromResource(ctx, ref); err != nil { return @@ -574,7 +554,7 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, // add this childs permissions pset := n.PermissionSet(ctx) node.AddPermissions(&np, &pset) - if ri, err := children[i].AsResourceInfo(ctx, &np, mdKeys, fieldMask, utils.IsRelativeReference(ref)); err == nil { + if ri, err := children[i].AsResourceInfo(ctx, &np, mdKeys, utils.IsRelativeReference(ref)); err == nil { finfos = append(finfos, ri) } } diff --git a/pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go b/pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go index 1fa374a867..7bc6fe1cb8 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs_concurrency_test.go @@ -111,7 +111,7 @@ var _ = Describe("Decomposed", func() { } if err := env.Fs.CreateDir(env.Ctx, ref); err != nil { Expect(err).To(MatchError(ContainSubstring("already exists"))) - rinfo, err := env.Fs.GetMD(env.Ctx, ref, nil, nil) + rinfo, err := env.Fs.GetMD(env.Ctx, ref, nil) Expect(err).ToNot(HaveOccurred()) Expect(rinfo).ToNot(BeNil()) } diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index e63ee5e1c2..4bbc7ed866 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -204,15 +204,11 @@ func ReadNode(ctx context.Context, lu PathLookup, spaceID, nodeID string, canLis } r.Exists = true - // TODO ReadNode should not check permissions if !canListDisabledSpace && r.IsDisabled() { // no permission = not found return nil, errtypes.NotFound(spaceID) } - // if current user cannot stat the root return not found? - // no for shares the root might be a different resource - // check if this is a space root if spaceID == nodeID { return r, nil @@ -588,7 +584,7 @@ func (n *Node) IsDir() bool { } // AsResourceInfo return the node as CS3 ResourceInfo -func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissions, mdKeys, fieldMask []string, returnBasename bool) (ri *provider.ResourceInfo, err error) { +func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissions, mdKeys []string, returnBasename bool) (ri *provider.ResourceInfo, err error) { sublog := appctx.GetLogger(ctx).With().Interface("node", n.ID).Logger() var fn string @@ -690,25 +686,15 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi mdKeysMap[k] = struct{}{} } - var returnAllMetadata bool + var returnAllKeys bool if _, ok := mdKeysMap["*"]; len(mdKeys) == 0 || ok { - returnAllMetadata = true + returnAllKeys = true } metadata := map[string]string{} - fieldMaskKeysMap := make(map[string]struct{}) - for _, k := range fieldMask { - fieldMaskKeysMap[k] = struct{}{} - } - - var returnAllFields bool - if _, ok := fieldMaskKeysMap["*"]; len(fieldMask) == 0 || ok { - returnAllFields = true - } - // read favorite flag for the current user - if _, ok := mdKeysMap[FavoriteKey]; returnAllMetadata || ok { + if _, ok := mdKeysMap[FavoriteKey]; returnAllKeys || ok { favorite := "" if u, ok := ctxpkg.ContextGetUser(ctx); ok { // the favorite flag is specific to the user, so we need to incorporate the userid @@ -729,8 +715,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi metadata[FavoriteKey] = favorite } // read locks - // FIXME move to fieldmask - if _, ok := mdKeysMap[LockdiscoveryKey]; returnAllMetadata || ok { + if _, ok := mdKeysMap[LockdiscoveryKey]; returnAllKeys || ok { if n.hasLocks(ctx) { err = readLocksIntoOpaque(ctx, n, ri) if err != nil { @@ -740,36 +725,21 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi } // share indicator - if _, ok := fieldMaskKeysMap["share-types"]; returnAllFields || ok { - granteeTypes := n.getGranteeTypes(ctx) - if len(granteeTypes) > 0 { - // TODO add optional property to CS3 ResourceInfo to transport grants? - var s strings.Builder - first := true - for _, t := range granteeTypes { - if !first { - s.WriteString(",") - } else { - first = false - } - s.WriteString(strconv.Itoa(int(t))) - } - ri.Opaque = utils.AppendPlainToOpaque(ri.Opaque, "share-types", s.String()) + if _, ok := mdKeysMap[ShareTypesKey]; returnAllKeys || ok { + if n.hasUserShares(ctx) { + metadata[ShareTypesKey] = UserShareType } } // checksums - // FIXME move to fieldmask - if _, ok := mdKeysMap[ChecksumsKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_FILE) && (returnAllMetadata || ok) { + if _, ok := mdKeysMap[ChecksumsKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_FILE) && (returnAllKeys || ok) { // TODO which checksum was requested? sha1 adler32 or md5? for now hardcode sha1? - // TODO make ResourceInfo carry multiple checksums readChecksumIntoResourceChecksum(ctx, nodePath, storageprovider.XSSHA1, ri) readChecksumIntoOpaque(ctx, nodePath, storageprovider.XSMD5, ri) readChecksumIntoOpaque(ctx, nodePath, storageprovider.XSAdler32, ri) } // quota - // FIXME move to fieldmask - if _, ok := mdKeysMap[QuotaKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_CONTAINER) && returnAllMetadata || ok { + if _, ok := mdKeysMap[QuotaKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_CONTAINER) && returnAllKeys || ok { if n.SpaceRoot != nil && n.SpaceRoot.InternalPath() != "" { readQuotaIntoOpaque(ctx, n.SpaceRoot.InternalPath(), ri) } @@ -787,7 +757,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi } // only read when key was requested k := attrs[i][len(xattrs.MetadataPrefix):] - if _, ok := mdKeysMap[k]; returnAllMetadata || ok { + if _, ok := mdKeysMap[k]; returnAllKeys || ok { if val, err := xattrs.Get(nodePath, attrs[i]); err == nil { metadata[k] = val } else { @@ -821,7 +791,7 @@ func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") case xattrs.IsNotExist(err): - appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not found") + appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") } @@ -843,7 +813,7 @@ func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *prov case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") case xattrs.IsNotExist(err): - appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not found") + appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") } @@ -1114,28 +1084,19 @@ func ReadBlobIDAttr(path string) (string, error) { return attr, nil } -func (n *Node) getGranteeTypes(ctx context.Context) []provider.GranteeType { - types := []provider.GranteeType{} - if g, err := n.ListGrantees(ctx); err == nil { - hasUserShares, hasGroupShares := false, false - for i := range g { - switch { - case !hasUserShares && strings.HasPrefix(g[i], xattrs.GrantUserAcePrefix): - hasUserShares = true - case !hasGroupShares && strings.HasPrefix(g[i], xattrs.GrantGroupAcePrefix): - hasGroupShares = true - case hasUserShares && hasGroupShares: - break - } - } - if hasUserShares { - types = append(types, provider.GranteeType_GRANTEE_TYPE_USER) - } - if hasGroupShares { - types = append(types, provider.GranteeType_GRANTEE_TYPE_GROUP) +func (n *Node) hasUserShares(ctx context.Context) bool { + g, err := n.ListGrantees(ctx) + if err != nil { + appctx.GetLogger(ctx).Error().Err(err).Msg("hasUserShares: listGrantees") + return false + } + + for i := range g { + if strings.HasPrefix(g[i], xattrs.GrantUserAcePrefix) { + return true } } - return types + return false } func parseMTime(v string) (t time.Time, err error) { diff --git a/pkg/storage/utils/decomposedfs/node/node_test.go b/pkg/storage/utils/decomposedfs/node/node_test.go index 9b6a460421..cfae71aadb 100644 --- a/pkg/storage/utils/decomposedfs/node/node_test.go +++ b/pkg/storage/utils/decomposedfs/node/node_test.go @@ -184,14 +184,14 @@ var _ = Describe("Node", func() { Describe("the Etag field", func() { It("is set", func() { perms := node.OwnerPermissions() - ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) Expect(len(ri.Etag)).To(Equal(34)) }) It("changes when the tmtime is set", func() { perms := node.OwnerPermissions() - ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) Expect(len(ri.Etag)).To(Equal(34)) before := ri.Etag @@ -199,7 +199,7 @@ var _ = Describe("Node", func() { tmtime := time.Now() Expect(n.SetTMTime(&tmtime)).To(Succeed()) - ri, err = n.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + ri, err = n.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) Expect(len(ri.Etag)).To(Equal(34)) Expect(ri.Etag).ToNot(Equal(before)) @@ -215,7 +215,7 @@ var _ = Describe("Node", func() { Expect(err).ToNot(HaveOccurred()) perms := node.OwnerPermissions() - ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) Expect(ri.Opaque).ToNot(BeNil()) Expect(ri.Opaque.Map["lock"]).ToNot(BeNil()) diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index 8dc69a8189..618f23f274 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -181,6 +181,13 @@ func (p *Permissions) HasPermission(ctx context.Context, n *Node, check func(*pr return check(perms), nil } + // determine root + /* + if err = n.FindStorageSpaceRoot(); err != nil { + return false, err + } + */ + // for an efficient group lookup convert the list of groups to a map // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! groupsMap := make(map[string]bool, len(u.Groups)) @@ -200,7 +207,7 @@ func (p *Permissions) HasPermission(ctx context.Context, n *Node, check func(*pr } } - // also check permissions on root, eg. for project spaces + // also check permissions on root, eg. for for project spaces return nodeHasPermission(ctx, cn, groupsMap, u.Id.OpaqueId, check), nil } diff --git a/pkg/storage/utils/decomposedfs/tree/tree_test.go b/pkg/storage/utils/decomposedfs/tree/tree_test.go index 7112e3fe74..4b85656d23 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree_test.go +++ b/pkg/storage/utils/decomposedfs/tree/tree_test.go @@ -302,13 +302,13 @@ var _ = Describe("Tree", func() { Expect(err).ToNot(HaveOccurred()) perms := node.OwnerPermissions() - riBefore, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + riBefore, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) err = env.Tree.Propagate(env.Ctx, file) Expect(err).ToNot(HaveOccurred()) - riAfter, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, []string{}, false) + riAfter, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, false) Expect(err).ToNot(HaveOccurred()) Expect(riAfter.Etag).ToNot(Equal(riBefore.Etag)) }) diff --git a/pkg/storage/utils/decomposedfs/upload_test.go b/pkg/storage/utils/decomposedfs/upload_test.go index 5f4276ed5b..0d13917e57 100644 --- a/pkg/storage/utils/decomposedfs/upload_test.go +++ b/pkg/storage/utils/decomposedfs/upload_test.go @@ -193,7 +193,7 @@ var _ = Describe("File uploads", func() { Expect(uploadIds["simple"]).ToNot(BeEmpty()) Expect(uploadIds["tus"]).ToNot(BeEmpty()) - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}) Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(0)) @@ -209,7 +209,7 @@ var _ = Describe("File uploads", func() { Expect(uploadIds["simple"]).ToNot(BeEmpty()) Expect(uploadIds["tus"]).ToNot(BeEmpty()) - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}) Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(0)) @@ -246,7 +246,7 @@ var _ = Describe("File uploads", func() { Expect(err).ToNot(HaveOccurred()) bs.AssertCalled(GinkgoT(), "Upload", mock.Anything, mock.Anything, mock.Anything) - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}) Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(1)) @@ -284,7 +284,7 @@ var _ = Describe("File uploads", func() { Expect(err).ToNot(HaveOccurred()) bs.AssertCalled(GinkgoT(), "Upload", mock.Anything, mock.Anything, mock.Anything) - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}) Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(1)) @@ -303,7 +303,7 @@ var _ = Describe("File uploads", func() { Expect(err).To(HaveOccurred()) - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}) Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(0)) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index c0f7c45e72..00bd59d232 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -792,7 +792,7 @@ func (fs *eosfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]*pr return grantList, nil } -func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { +func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { log := appctx.GetLogger(ctx) log.Info().Msg("eosfs: get md for ref:" + ref.String()) @@ -887,7 +887,7 @@ func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string return fs.convertToFileReference(ctx, eosFileInfo) } -func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { +func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { p, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "eosfs: error resolving reference") @@ -1444,7 +1444,7 @@ func (fs *eosfs) ListRevisions(ctx context.Context, ref *provider.Reference) ([] // We need to access the revisions for a non-home reference. // We'll get the owner of the particular resource and impersonate them // if we have access to it. - md, err := fs.GetMD(ctx, ref, nil, nil) + md, err := fs.GetMD(ctx, ref, nil) if err != nil { return nil, err } @@ -1487,7 +1487,7 @@ func (fs *eosfs) DownloadRevision(ctx context.Context, ref *provider.Reference, // We need to access the revisions for a non-home reference. // We'll get the owner of the particular resource and impersonate them // if we have access to it. - md, err := fs.GetMD(ctx, ref, nil, nil) + md, err := fs.GetMD(ctx, ref, nil) if err != nil { return nil, err } @@ -1520,7 +1520,7 @@ func (fs *eosfs) RestoreRevision(ctx context.Context, ref *provider.Reference, r // We need to access the revisions for a non-home reference. // We'll get the owner of the particular resource and impersonate them // if we have access to it. - md, err := fs.GetMD(ctx, ref, nil, nil) + md, err := fs.GetMD(ctx, ref, nil) if err != nil { return err } @@ -1568,7 +1568,7 @@ func (fs *eosfs) ListRecycle(ctx context.Context, ref *provider.Reference, key, // We need to access the recycle bin for a non-home reference. // We'll get the owner of the particular resource and impersonate them // if we have access to it. - md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil, nil) + md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil) if err != nil { return nil, err } @@ -1619,7 +1619,7 @@ func (fs *eosfs) RestoreRecycleItem(ctx context.Context, ref *provider.Reference // We need to access the recycle bin for a non-home reference. // We'll get the owner of the particular resource and impersonate them // if we have access to it. - md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil, nil) + md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil) if err != nil { return err } diff --git a/pkg/storage/utils/eosfs/spaces.go b/pkg/storage/utils/eosfs/spaces.go index 78d1cc20a0..477265eb15 100644 --- a/pkg/storage/utils/eosfs/spaces.go +++ b/pkg/storage/utils/eosfs/spaces.go @@ -228,7 +228,7 @@ func (fs *eosfs) listProjectStorageSpaces(ctx context.Context, user *userpb.User for rows.Next() { var name, relPath string if err = rows.Scan(&name, &relPath); err == nil { - info, err := fs.GetMD(ctx, &provider.Reference{Path: relPath}, []string{}, nil) + info, err := fs.GetMD(ctx, &provider.Reference{Path: relPath}, []string{}) if err == nil { if (spaceID == "" || spaceID == info.Id.OpaqueId) && (spacePath == "" || spacePath == relPath) { // If the request was for a relative ref, return just the base path diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go index a013d7d0ba..452432b276 100644 --- a/pkg/storage/utils/localfs/localfs.go +++ b/pkg/storage/utils/localfs/localfs.go @@ -920,7 +920,7 @@ func (fs *localfs) moveReferences(ctx context.Context, oldName, newName string) return nil } -func (fs *localfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { +func (fs *localfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { fn, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "localfs: error resolving ref") @@ -961,7 +961,7 @@ func (fs *localfs) getMDShareFolder(ctx context.Context, p string, mdKeys []stri return fs.convertToFileReference(ctx, md, fn, mdKeys) } -func (fs *localfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) ([]*provider.ResourceInfo, error) { +func (fs *localfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) { fn, err := fs.resolve(ctx, ref) if err != nil { return nil, errors.Wrap(err, "localfs: error resolving ref") diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index 1f361564c9..aa77ab3f5d 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -1289,6 +1289,16 @@ _ocs: api compatibility, return correct status code_ - [apiShareOperationsToShares2/shareAccessByID.feature:162](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares2/shareAccessByID.feature#L162) - [apiShareOperationsToShares2/shareAccessByID.feature:163](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares2/shareAccessByID.feature#L163) +#### [[OC-storage] share-types field empty for shared file folder in webdav response](https://github.com/owncloud/ocis/issues/2144) +- [apiWebdavProperties2/getFileProperties.feature:215](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L215) +- [apiWebdavProperties2/getFileProperties.feature:216](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L216) +- [apiWebdavProperties2/getFileProperties.feature:221](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L221) + +#### [Different share permissions provides varying roles in oc10 and ocis](https://github.com/owncloud/ocis/issues/1277) +- [apiWebdavProperties2/getFileProperties.feature:275](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L275) +- [apiWebdavProperties2/getFileProperties.feature:276](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L276) +- [apiWebdavProperties2/getFileProperties.feature:281](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L281) + #### [Cannot move folder/file from one received share to another](https://github.com/owncloud/ocis/issues/2442) - [apiShareUpdateToShares/updateShare.feature:242](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareUpdateToShares/updateShare.feature#L242) - [apiShareUpdateToShares/updateShare.feature:196](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareUpdateToShares/updateShare.feature#L196) @@ -1397,6 +1407,9 @@ moving outside of the Shares folder gives 501 Not Implemented. - [apiWebdavOperations/search.feature:289](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L289) - [apiWebdavOperations/search.feature:314](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L314) +#### [Incorrect response while listing resources of a folder with depth infinity](https://github.com/owncloud/ocis/issues/3073) +- [apiWebdavOperations/listFiles.feature:182](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L182) + #### [Cannot disable the dav propfind depth infinity for resources](https://github.com/owncloud/ocis/issues/3720) - [apiWebdavOperations/listFiles.feature:398](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L398) - [apiWebdavOperations/listFiles.feature:399](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L399) diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index 6b53ac9c6b..408f81139e 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -1307,6 +1307,16 @@ _ocs: api compatibility, return correct status code_ - [apiShareOperationsToShares2/shareAccessByID.feature:162](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares2/shareAccessByID.feature#L162) - [apiShareOperationsToShares2/shareAccessByID.feature:163](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares2/shareAccessByID.feature#L163) +#### [[OC-storage] share-types field empty for shared file folder in webdav response](https://github.com/owncloud/ocis/issues/2144) +- [apiWebdavProperties2/getFileProperties.feature:215](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L215) +- [apiWebdavProperties2/getFileProperties.feature:216](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L216) +- [apiWebdavProperties2/getFileProperties.feature:221](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L221) + +#### [Different share permissions provides varying roles in oc10 and ocis](https://github.com/owncloud/ocis/issues/1277) +- [apiWebdavProperties2/getFileProperties.feature:275](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L275) +- [apiWebdavProperties2/getFileProperties.feature:276](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L276) +- [apiWebdavProperties2/getFileProperties.feature:281](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties2/getFileProperties.feature#L281) + #### [Cannot move folder/file from one received share to another](https://github.com/owncloud/ocis/issues/2442) - [apiShareUpdateToShares/updateShare.feature:242](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareUpdateToShares/updateShare.feature#L242) - [apiShareUpdateToShares/updateShare.feature:196](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareUpdateToShares/updateShare.feature#L196) @@ -1400,6 +1410,9 @@ _ocs: api compatibility, return correct status code_ - [apiWebdavOperations/search.feature:289](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L289) - [apiWebdavOperations/search.feature:314](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/search.feature#L314) +#### [Incorrect response while listing resources of a folder with depth infinity](https://github.com/owncloud/ocis/issues/3073) +- [apiWebdavOperations/listFiles.feature:182](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L182) + #### [Cannot disable the dav propfind depth infinity for resources](https://github.com/owncloud/ocis/issues/3720) - [apiWebdavOperations/listFiles.feature:398](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L398) - [apiWebdavOperations/listFiles.feature:399](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavOperations/listFiles.feature#L399)