diff --git a/changelog/unreleased/skip-space-lookup-on-space-propfind.md b/changelog/unreleased/skip-space-lookup-on-space-propfind.md new file mode 100644 index 0000000000..63600068a2 --- /dev/null +++ b/changelog/unreleased/skip-space-lookup-on-space-propfind.md @@ -0,0 +1,8 @@ +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 1b6b3b8dcb..23dfa13e9c 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -837,7 +837,12 @@ 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}) + return c.Stat(ctx, &provider.StatRequest{ + Opaque: req.Opaque, + Ref: ref, + ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, + FieldMask: req.FieldMask, + }) } func (s *svc) ListContainerStream(_ *provider.ListContainerStreamRequest, _ gateway.GatewayAPI_ListContainerStreamServer) error { @@ -858,6 +863,7 @@ 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 916135446b..978006fd32 100644 --- a/internal/grpc/services/gateway/storageprovidercache.go +++ b/internal/grpc/services/gateway/storageprovidercache.go @@ -229,24 +229,37 @@ 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 []string) string { +func statKey(user *userpb.User, ref *provider.Reference, metaDataKeys, fieldMaskPaths []string) string { if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" { return "" } - key := "uid:" + user.Id.OpaqueId + "!sid:" + ref.ResourceId.StorageId + "!oid:" + ref.ResourceId.OpaqueId + "!path:" + ref.Path + 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) for _, k := range metaDataKeys { - key += "!mdk:" + k + key.WriteString("!mdk:") + key.WriteString(k) + } + for _, p := range fieldMaskPaths { + key.WriteString("!fmp:") + key.WriteString(p) } - return key + return key.String() } // 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.Ref, in.ArbitraryMetadataKeys) + key := statKey(ctxpkg.ContextMustGetUser(ctx), in.GetRef(), in.GetArbitraryMetadataKeys(), in.GetFieldMask().GetPaths()) 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 2ff40af8ef..ef108120e3 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -24,7 +24,6 @@ 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" @@ -369,10 +368,10 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } var receivedShares []*collaboration.ReceivedShare - var shareMd map[string]share.Metadata + var shareInfo map[string]*provider.ResourceInfo var err error if fetchShares { - receivedShares, shareMd, err = s.fetchShares(ctx) + receivedShares, shareInfo, err = s.fetchShares(ctx) if err != nil { return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } @@ -389,13 +388,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora OpaqueId: utils.ShareStorageProviderID, } if spaceID == nil || isShareJailRoot(spaceID) { - earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareMd) + earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareInfo) var opaque *typesv1beta1.Opaque var mtime *typesv1beta1.Timestamp if earliestShare != nil { - if md, ok := shareMd[earliestShare.Id.OpaqueId]; ok { - mtime = md.Mtime - opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) + if info, ok := shareInfo[earliestShare.Id.OpaqueId]; ok { + mtime = info.Mtime + opaque = utils.AppendPlainToOpaque(opaque, "etag", info.Etag) } } // only display the shares jail if we have accepted shares @@ -424,20 +423,16 @@ 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, + Root: root, + RootInfo: shareInfo[receivedShare.Share.Id.OpaqueId], } res.StorageSpaces = append(res.StorageSpaces, space) @@ -465,9 +460,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } } var opaque *typesv1beta1.Opaque - if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) - } else { + if _, ok := shareInfo[receivedShare.Share.Id.OpaqueId]; !ok { // we could not stat the share, skip it continue } @@ -490,7 +483,8 @@ 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, + Root: root, + RootInfo: shareInfo[receivedShare.Share.Id.OpaqueId], } // TODO in the future the spaces registry will handle the alias for share spaces. @@ -711,7 +705,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 @@ -1031,7 +1025,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]share.Metadata, error) { +func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedShare, map[string]*provider.ResourceInfo, error) { lsRes, err := s.sharesProviderClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ // FIXME filter by received shares for resource id - listing all shares is tooo expensive! }) @@ -1042,7 +1036,7 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest") } - shareMetaData := make(map[string]share.Metadata, len(lsRes.Shares)) + shareMetaData := make(map[string]*provider.ResourceInfo, len(lsRes.Shares)) for _, rs := range lsRes.Shares { // only stat accepted shares if rs.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { @@ -1063,13 +1057,13 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha Msg("ListRecievedShares: failed to stat the resource") continue } - shareMetaData[rs.Share.Id.OpaqueId] = share.Metadata{ETag: sRes.Info.Etag, Mtime: sRes.Info.Mtime} + shareMetaData[rs.Share.Id.OpaqueId] = sRes.Info } return lsRes.Shares, shareMetaData, nil } -func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd map[string]share.Metadata) (earliestShare *collaboration.Share, atLeastOneAccepted bool) { +func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareInfo map[string]*provider.ResourceInfo) (earliestShare *collaboration.Share, atLeastOneAccepted bool) { for _, rs := range receivedShares { var hasCurrentMd bool var hasEarliestMd bool @@ -1081,10 +1075,10 @@ func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd ma // We cannot assume that every share has metadata if current.Id != nil { - _, hasCurrentMd = shareMd[current.Id.OpaqueId] + _, hasCurrentMd = shareInfo[current.Id.OpaqueId] } if earliestShare != nil && earliestShare.Id != nil { - _, hasEarliestMd = shareMd[earliestShare.Id.OpaqueId] + _, hasEarliestMd = shareInfo[earliestShare.Id.OpaqueId] } switch { @@ -1093,10 +1087,10 @@ func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd ma // ignore if one of the shares has no metadata case !hasEarliestMd || !hasCurrentMd: continue - case shareMd[current.Id.OpaqueId].Mtime.Seconds > shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds: + case shareInfo[current.Id.OpaqueId].Mtime.Seconds > shareInfo[earliestShare.Id.OpaqueId].Mtime.Seconds: earliestShare = current - 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: + 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: earliestShare = current } } diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 63c955043d..ce34ddcbec 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{}) + md, err := s.storage.GetMD(ctx, req.Ref, []string{}, []string{"id"}) 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.Ref, req.ArbitraryMetadataKeys) + md, err := s.storage.GetMD(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) 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.Ref, req.ArbitraryMetadataKeys) + mds, err := s.storage.ListFolder(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) 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.Ref, req.ArbitraryMetadataKeys) + mds, err := s.storage.ListFolder(ctx, req.GetRef(), req.GetArbitraryMetadataKeys(), req.GetFieldMask().GetPaths()) 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 168d398f64..f28432da6d 100644 --- a/internal/http/services/owncloud/ocdav/delete.go +++ b/internal/http/services/owncloud/ocdav/delete.go @@ -96,8 +96,7 @@ 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) - // TODO path might be empty or relative... - m := fmt.Sprintf("Resource %v not found", ref.Path) + m := "Resource not found" // mimic the oc10 error message b, err := errors.Marshal(http.StatusNotFound, m, "") errors.HandleWebdavError(&log, w, b, err) case rpc.Code_CODE_PERMISSION_DENIED: @@ -119,12 +118,11 @@ 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 dont leak existence of a file + // return not found error so we do not 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 - // TODO path might be empty or relative... - m = fmt.Sprintf("%s not fount", ref.Path) + m = "Resource not found" // mimic the oc10 error message } 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 99500580f0..f7dc30cfcc 100644 --- a/internal/http/services/owncloud/ocdav/locks.go +++ b/internal/http/services/owncloud/ocdav/locks.go @@ -400,23 +400,12 @@ func (s *svc) handleSpacesLock(w http.ResponseWriter, r *http.Request, spaceID s span.SetAttributes(attribute.String("component", "ocdav")) - client, err := s.getClient() + ref, err := spacelookup.MakeStorageSpaceReference(spaceID, r.URL.Path) if err != nil { - return http.StatusInternalServerError, err + return http.StatusBadRequest, fmt.Errorf("invalid space id") } - // 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) + 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 6a16925e3e..e6a318b4e9 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 dont leak existence of a file + // return not found error so we do not 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(sRes.Status.Message) + return http.StatusNotFound, fmt.Errorf("Resource not found") } 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 52a95dbb73..df90d295d4 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -36,6 +36,7 @@ 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" @@ -45,16 +46,18 @@ 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" - _spaceTypeProject = "project" + tracerName = "ocdav" ) type countingReader struct { @@ -199,6 +202,7 @@ 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") @@ -211,28 +215,12 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns return } - 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) + resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, sublog) if !ok { // getResourceInfos handles responses in case of an error so we can just return here. return } - p.propfindResponse(ctx, w, r, ns, root.SpaceType, pf, sendTusHeaders, resourceInfos, sublog) + p.propfindResponse(ctx, w, r, ns, pf, sendTusHeaders, resourceInfos, sublog) } // HandleSpacesPropfind handles a spaces based propfind request @@ -241,10 +229,15 @@ 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() - client, err := p.getClient() + dh := r.Header.Get(net.HeaderDepth) + + depth, err := net.ParseDepth(dh) if err != nil { - sublog.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) + 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) return } @@ -255,35 +248,121 @@ func (p *Handler) HandleSpacesPropfind(w http.ResponseWriter, r *http.Request, s return } - // retrieve a specific storage space - space, rpcStatus, err := spacelookup.LookUpStorageSpaceByID(ctx, client, spaceID) + ref, err := spacelookup.MakeStorageSpaceReference(spaceID, r.URL.Path) if err != nil { - sublog.Error().Err(err).Msg("error looking up the space by id") - w.WriteHeader(http.StatusInternalServerError) + 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 } - if rpcStatus.Code != rpc.Code_CODE_OK { - errors.HandleErrorStatus(&sublog, w, rpcStatus) + client, err := p.getClient() + if err != nil { + sublog.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) return } - 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. + 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) + 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...) + } // 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, "", space.SpaceType, pf, sendTusHeaders, resourceInfos, sublog) + p.propfindResponse(ctx, w, r, "", pf, sendTusHeaders, resourceInfos, sublog) } -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) { +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) { ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() @@ -302,20 +381,27 @@ 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" { - 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{}{} + // 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 } - } 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, spaceType, linkshares) + propRes, err := MultistatusResponse(ctx, &pf, resourceInfos, p.PublicURL, namespace, linkshares) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) @@ -337,10 +423,11 @@ 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, space *provider.StorageSpace, ref *provider.Reference, metadataKeys []string) (*provider.ResourceInfo, *rpc.Status, error) { +func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference, metadataKeys, fieldMaskPaths []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 { @@ -349,17 +436,21 @@ 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, spacesPropfind bool, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) { +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() + 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 { @@ -368,23 +459,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r return nil, false, false } - 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])) - } - } - } + metadataKeys, fieldMaskPaths := metadataKeys(pf) // 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!) @@ -395,27 +470,41 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r spaceMap = make(map[*provider.ResourceInfo]spaceData, len(spaces)) ) for _, space := range spaces { - spacePath, ok := getMountPoint(*space) - if !ok { + spacePath := "" + if spacePath = utils.ReadPlainFromOpaque(space.Opaque, "path"); spacePath == "" { 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 ... - 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 + // 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 + } } // adjust 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) - } + info.Path = filepath.Join(spacePath, spaceRef.Path) spaceMap[info] = spaceData{Ref: spaceRef, SpaceType: space.SpaceType} @@ -470,12 +559,12 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r childInfos := map[string]*provider.ResourceInfo{} for spaceInfo, spaceData := range spaceMap { switch { - case !spacesPropfind && spaceInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != net.DepthInfinity: + case 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) && (spacesPropfind || spaceData.SpaceType != "virtual"): + case strings.HasPrefix(requestPath, spaceInfo.Path) && spaceData.SpaceType != "virtual": req := &provider.ListContainerRequest{ Ref: spaceData.Ref, ArbitraryMetadataKeys: metadataKeys, @@ -536,9 +625,6 @@ 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]) } @@ -570,6 +656,125 @@ 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, @@ -604,15 +809,6 @@ 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: @@ -670,10 +866,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, spaceType string, linkshares map[string]struct{}) ([]byte, error) { +func MultistatusResponse(ctx context.Context, pf *XML, mds []*provider.ResourceInfo, publicURL, ns 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, spaceType, linkshares) + res, err := mdToPropResponse(ctx, pf, mds[i], publicURL, ns, linkshares) if err != nil { return nil, err } @@ -692,7 +888,12 @@ 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, spaceType string, linkshares map[string]struct{}) (*ResponseXML, error) { +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() + sublog := appctx.GetLogger(ctx).With().Interface("md", md).Str("ns", ns).Logger() md.Path = strings.TrimPrefix(md.Path, ns) @@ -716,6 +917,7 @@ 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 { @@ -726,8 +928,8 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p sublog.Error().Err(err).Msg("could not unmarshal link json") } } - if md.Opaque.Map["quota"] != nil && md.Opaque.Map["quota"].Decoder == "plain" { - quota = string(md.Opaque.Map["quota"].Value) + if quota = utils.ReadPlainFromOpaque(md.Opaque, "quota"); quota == "" { + quota = net.PropQuotaUnknown } if md.Opaque.Map["lock"] != nil && md.Opaque.Map["lock"].Decoder == "json" { lock = &provider.Lock{} @@ -736,13 +938,18 @@ 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) - isShared := spaceType != _spaceTypeProject && !net.IsCurrentUserOwner(ctx, md.Owner) + if md.Space != nil && md.Space.SpaceType != "grant" && utils.ResourceIDEqual(md.Space.Root, md.Id) { + // a space root is never shared + shareTypes = "" + } 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, @@ -750,7 +957,6 @@ 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 @@ -1026,14 +1232,19 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p } else { propstatNotFound.Prop = append(propstatNotFound.Prop, prop.NotFound("oc:checksums")) } - case "share-types": // desktop + case "share-types": // used to render share indicators to share owners var types strings.Builder - k := md.GetArbitraryMetadata() - amd := k.GetMetadata() - if amdv, ok := amd[metadataKeyOf(&pf.Prop[i])]; ok { - types.WriteString("") - types.WriteString(amdv) - types.WriteString("") + + 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") + } } if md.Id != nil { @@ -1314,12 +1525,17 @@ func (c *countingReader) Read(p []byte) (int, error) { } func metadataKeyOf(n *xml.Name) string { - switch { - case n.Space == net.NsDav && n.Local == "quota-available-bytes": - return "quota" - default: - return fmt.Sprintf("%s/%s", n.Space, n.Local) + 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" + } } + return fmt.Sprintf("%s/%s", n.Space, n.Local) } // UnmarshalXML appends the property names enclosed within start to pn. @@ -1353,15 +1569,3 @@ 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 daafb791e6..0816ed7ef6 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go @@ -34,6 +34,7 @@ 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" @@ -64,7 +65,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 (ref.ResourceId.GetOpaqueId() == "" || req.Ref.ResourceId.GetOpaqueId() == ref.ResourceId.GetOpaqueId()) && + return utils.ResourceIDEqual(req.Ref.ResourceId, ref.ResourceId) && (ref.Path == "" || req.Ref.Path == ref.Path) })).Return(&sprovider.StatResponse{ Status: status.NewOK(ctx), @@ -73,7 +74,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 := (ref.ResourceId.GetOpaqueId() == "" || req.Ref.ResourceId.GetOpaqueId() == ref.ResourceId.GetOpaqueId()) && + match := utils.ResourceIDEqual(req.Ref.ResourceId, ref.ResourceId) && (ref.Path == "" || req.Ref.Path == ref.Path) return match })).Return(&sprovider.ListContainerResponse{ @@ -82,6 +83,20 @@ 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{ @@ -91,8 +106,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "foospace"}, - Root: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "foospace!root"}, + Root: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Name: "foospace", } fooquxspace = &sprovider.StorageSpace{ @@ -104,8 +119,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooquxspace"}, - Root: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooquxspace!root"}, + Root: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, Name: "fooquxspace", } fooFileShareSpace = &sprovider.StorageSpace{ @@ -117,8 +132,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace"}, - Root: &sprovider.ResourceId{OpaqueId: "sharedfile"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace!sharedfile"}, + Root: &sprovider.ResourceId{StorageId: "fooFileShareSpace", OpaqueId: "sharedfile"}, Name: "fooFileShareSpace", } fooFileShare2Space = &sprovider.StorageSpace{ @@ -130,8 +145,8 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace2"}, - Root: &sprovider.ResourceId{OpaqueId: "sharedfile2"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooFileShareSpace2!sharedfile2"}, + Root: &sprovider.ResourceId{StorageId: "fooFileShareSpace2", OpaqueId: "sharedfile2"}, Name: "fooFileShareSpace2", } fooDirShareSpace = &sprovider.StorageSpace{ @@ -143,80 +158,131 @@ var _ = Describe("Propfind", func() { }, }, }, - Id: &sprovider.StorageSpaceId{OpaqueId: "fooDirShareSpace"}, - Root: &sprovider.ResourceId{OpaqueId: "shareddir"}, + Id: &sprovider.StorageSpaceId{OpaqueId: "fooDirShareSpace!shareddir"}, + Root: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, Name: "fooDirShareSpace", } - ) - 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: "."}, + // 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: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{OpaqueId: "foospaceroot", StorageId: "foospaceroot"}, + Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, Path: ".", Size: uint64(131), }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "root"}, 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{OpaqueId: "foospaceroot"}, Path: "./bar"}, + 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: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "foospacebar"}, + Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "./bar", Size: uint64(100), }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "foospaceroot"}, Path: "./dir"}, + 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: "."}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "dirent"}, + Id: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "direntry"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "entry", Size: 30, }, }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot"}, Path: "."}, + // 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: "."}, &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{OpaqueId: "fooquxspaceroot"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "root"}, Path: "."}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{OpaqueId: "fooquxspaceroot", StorageId: "fooquxspaceroot"}, + Id: &sprovider.ResourceId{StorageId: "fooquxspace", OpaqueId: "quux"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - Path: "quux", + Path: "./quux", Size: 1000, }, }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "sharedfile"}, Path: "."}, + // 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: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{OpaqueId: "sharedfile", StorageId: "sharedfile"}, + Id: &sprovider.ResourceId{StorageId: "fooFileShareSpace", OpaqueId: "sharedfile"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: ".", Size: uint64(2000), @@ -224,28 +290,34 @@ var _ = Describe("Propfind", func() { Etag: "1", }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "sharedfile2"}, Path: "."}, + // 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: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{OpaqueId: "sharedfile2", StorageId: "sharedfile2"}, + Id: &sprovider.ResourceId{StorageId: "fooFileShareSpace2", OpaqueId: "sharedfile2"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: ".", Size: uint64(2500), Mtime: &typesv1beta1.Timestamp{Seconds: 2}, Etag: "2", }) - mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "shareddir"}, Path: "."}, + + // 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: "."}, &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{OpaqueId: "shareddir", StorageId: "shareddir"}, + Id: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, Path: ".", Size: uint64(1500), Mtime: &typesv1beta1.Timestamp{Seconds: 3}, Etag: "3", }) - mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{OpaqueId: "shareddir"}, Path: "."}, + mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddir"}, Path: "."}, []*sprovider.ResourceInfo{ { - Id: &sprovider.ResourceId{OpaqueId: "shareddir", StorageId: "shareddir"}, + Id: &sprovider.ResourceId{StorageId: "fooDirShareSpace", OpaqueId: "shareddirsomething"}, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Path: "something", Size: 1500, @@ -261,11 +333,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 == "foospacebar": + case term != nil && term.ResourceId != nil && term.ResourceId.OpaqueId == "bar": shares = []*link.PublicShare{ { Id: &link.PublicShareId{OpaqueId: "share1"}, - ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "foospacebar"}, + ResourceId: &sprovider.ResourceId{StorageId: "foospace", OpaqueId: "bar"}, }, } default: @@ -299,6 +371,13 @@ 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() { @@ -609,27 +688,32 @@ var _ = Describe("Propfind", func() { }) Describe("HandleSpacesPropfind", func() { - JustBeforeEach(func() { - client.On("ListStorageSpaces", mock.Anything, mock.Anything).Return( - func(_ context.Context, req *sprovider.ListStorageSpacesRequest, _ ...grpc.CallOption) *sprovider.ListStorageSpacesResponse { - var spaces []*sprovider.StorageSpace + /* + JustBeforeEach(func() { + client.On("Stat", mock.Anything, mock.Anything).Return(func(_ context.Context, req *sprovider.StatRequest, _ ...grpc.CallOption) *sprovider.StatResponse { switch { - case req.Filters[0].Term.(*sprovider.ListStorageSpacesRequest_Filter_Id).Id.OpaqueId == "foospace": - spaces = []*sprovider.StorageSpace{foospace} + 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: ".", + }, + } default: - spaces = []*sprovider.StorageSpace{} - } - return &sprovider.ListStorageSpacesResponse{ - Status: status.NewOK(ctx), - StorageSpaces: spaces, + return &sprovider.StatResponse{ + Status: status.NewNotFound(ctx, "not found"), + } } }, nil) - }) + }) + */ It("handles invalid space ids", func() { - client.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&sprovider.ListStorageSpacesResponse{ - Status: status.NewOK(ctx), - StorageSpaces: []*sprovider.StorageSpace{}, + client.On("Stat", mock.Anything, mock.Anything).Return(&sprovider.StatResponse{ + Status: status.NewNotFound(ctx, "not found"), }, nil) rr := httptest.NewRecorder() @@ -641,12 +725,15 @@ 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") + handler.HandleSpacesPropfind(rr, req, "foospace!root") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -654,19 +741,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/")) + Expect(root.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/")) 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/bar")) + Expect(bar.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/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/baz")) + Expect(baz.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/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/dir/")) + Expect(dir.Href).To(Equal("http:/127.0.0.1:3000/foospace%21root/dir/")) Expect(string(dir.Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("30")) }) @@ -676,7 +763,7 @@ var _ = Describe("Propfind", func() { Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace") + handler.HandleSpacesPropfind(rr, req, "foospace!root") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -686,30 +773,19 @@ 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", "/baz", strings.NewReader("")) + req, err := http.NewRequest("GET", "/dir", strings.NewReader("")) Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace") + handler.HandleSpacesPropfind(rr, req, "foospace!root") 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("50")) - Expect(string(res.Responses[1].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("50")) + 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")) }) It("includes all the thingsā„¢ when depth is infinity", func() { @@ -719,7 +795,7 @@ var _ = Describe("Propfind", func() { Expect(err).ToNot(HaveOccurred()) req = req.WithContext(ctx) - handler.HandleSpacesPropfind(rr, req, "foospace") + handler.HandleSpacesPropfind(rr, req, "foospace!root") Expect(rr.Code).To(Equal(http.StatusMultiStatus)) res, _, err := readResponse(rr.Result().Body) @@ -731,11 +807,11 @@ var _ = Describe("Propfind", func() { paths = append(paths, r.Href) } Expect(paths).To(ConsistOf( - "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", + "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", )) }) }) diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index b4c9e948f3..0714bf0a48 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -180,6 +180,13 @@ 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}) @@ -188,21 +195,19 @@ 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 dont leak existence of a file + // return not found error so we do not 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 } - errors.HandleErrorStatus(&log, w, res.Status) + 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) return nil, nil, false } if key == "http://owncloud.org/ns/favorite" { @@ -229,6 +234,13 @@ 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}) @@ -237,21 +249,19 @@ 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 dont leak existence of a file + // return not found error so we don't 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 } - errors.HandleErrorStatus(&log, w, res.Status) + 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) return nil, nil, false } diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index fc19def345..da8a341ed4 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 568f8cd93b..fe7fab892b 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -20,7 +20,6 @@ package ocdav import ( "context" - "fmt" "net/http" "path" "strconv" @@ -233,11 +232,13 @@ 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 dont leak existence of a file + // return not found error so we do not 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) + } + if status == http.StatusNotFound { + m = "Resource not found" // mimic the oc10 error message } 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 1c93756072..b5ad80a537 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 c7cbc7eb78..a7b4afe132 100644 --- a/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go +++ b/internal/http/services/owncloud/ocdav/spacelookup/spacelookup.go @@ -31,6 +31,7 @@ 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: @@ -98,12 +99,13 @@ func LookUpStorageSpacesForPathWithChildren(ctx context.Context, client gateway. // TODO use ListContainerStream to listen for changes // retrieve a specific storage space lSSReq := &storageProvider.ListStorageSpacesRequest{ - Opaque: &typesv1beta1.Opaque{ - Map: map[string]*typesv1beta1.OpaqueEntry{ - "path": {Decoder: "plain", Value: []byte(path)}, - "withChildMounts": {Decoder: "plain", Value: []byte("true")}, - }}, + // get all fields, including root_info + FieldMask: &fieldmaskpb.FieldMask{Paths: []string{"*"}}, } + // 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 37b2d935d4..24e4696ff9 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 1813a8e732..3e46f65104 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) (*provider.ResourceInfo, error) { - res, err := w.FS.GetMD(ctx, ref, mdKeys) +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) 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 []string) ([]*provider.ResourceInfo, error) { - res, err := w.FS.ListFolder(ctx, ref, mdKeys) +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) if err != nil { return nil, err } diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index bf247a1c28..3c7cad877f 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) (*provider.ResourceInfo, error) { - res, err := w.FS.GetMD(ctx, ref, mdKeys) +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) 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 []string) ([]*provider.ResourceInfo, error) { - res, err := w.FS.ListFolder(ctx, ref, mdKeys) +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) 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) + res, err := w.FS.GetMD(ctx, ref, nil, 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 356545bab9..77f6960cc5 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); err != nil { + if md, err = fs.GetMD(ctx, ref, nil, []string{"size", "mimetype"}); 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 94cf4be949..bd75318d5f 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -301,7 +301,8 @@ func (nc *StorageDriver) Move(ctx context.Context, oldRef, newRef *provider.Refe } // GetMD as defined in the storage.FS interface -func (nc *StorageDriver) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string) (*provider.ResourceInfo, error) { +// TODO forward fieldMask +func (nc *StorageDriver) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) { type paramsObj struct { Ref *provider.Reference `json:"ref"` MdKeys []string `json:"mdKeys"` @@ -331,7 +332,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 []string) ([]*provider.ResourceInfo, error) { +func (nc *StorageDriver) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []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 4852eb3122..d74dc693ff 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) + result, err := nc.GetMD(ctx, ref, mdKeys, nil) 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) + results, err := nc.ListFolder(ctx, ref, mdKeys, []string{}) 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 86d5e5deef..41e0010d27 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) (*provider.ResourceInfo, error) { +func (fs *owncloudsqlfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []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 []string) ([]*provider.ResourceInfo, error) { +func (fs *owncloudsqlfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []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 a174003bea..9459f3751c 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) (*provider.ResourceInfo, error) { +func (fs *s3FS) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []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 []string) ([]*provider.ResourceInfo, error) { +func (fs *s3FS) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []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 98068b715c..d02b912b5f 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 []string) (*provider.ResourceInfo, error) - ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, 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) 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 f4ec758038..39a8694432 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -173,7 +173,8 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) ( return 0, 0, 0, errtypes.PermissionDenied(n.ID) } - ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, true) + // FIXME move treesize & quota to fieldmask + ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, []string{}, true) if err != nil { return 0, 0, 0, err } @@ -291,6 +292,7 @@ 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) } @@ -498,7 +500,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) (ri *provider.ResourceInfo, err error) { +func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (ri *provider.ResourceInfo, err error) { var node *node.Node if node, err = fs.lu.NodeFromResource(ctx, ref); err != nil { return @@ -517,11 +519,29 @@ func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKe return nil, errtypes.PermissionDenied(node.ID) } - return node.AsResourceInfo(ctx, &rp, mdKeys, utils.IsRelativeReference(ref)) + 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 } // ListFolder returns a list of resources in the specified folder -func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) (finfos []*provider.ResourceInfo, err error) { +func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (finfos []*provider.ResourceInfo, err error) { var n *node.Node if n, err = fs.lu.NodeFromResource(ctx, ref); err != nil { return @@ -554,7 +574,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, utils.IsRelativeReference(ref)); err == nil { + if ri, err := children[i].AsResourceInfo(ctx, &np, mdKeys, fieldMask, 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 7bc6fe1cb8..1fa374a867 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) + rinfo, err := env.Fs.GetMD(env.Ctx, ref, nil, 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 eae80b1ce1..1f3ebb4c35 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -201,11 +201,15 @@ 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 @@ -581,7 +585,7 @@ func (n *Node) IsDir() bool { } // AsResourceInfo return the node as CS3 ResourceInfo -func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissions, mdKeys []string, returnBasename bool) (ri *provider.ResourceInfo, err error) { +func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissions, mdKeys, fieldMask []string, returnBasename bool) (ri *provider.ResourceInfo, err error) { sublog := appctx.GetLogger(ctx).With().Interface("node", n.ID).Logger() var fn string @@ -679,15 +683,25 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi mdKeysMap[k] = struct{}{} } - var returnAllKeys bool + var returnAllMetadata bool if _, ok := mdKeysMap["*"]; len(mdKeys) == 0 || ok { - returnAllKeys = true + returnAllMetadata = 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]; returnAllKeys || ok { + if _, ok := mdKeysMap[FavoriteKey]; returnAllMetadata || ok { favorite := "" if u, ok := ctxpkg.ContextGetUser(ctx); ok { // the favorite flag is specific to the user, so we need to incorporate the userid @@ -708,7 +722,8 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi metadata[FavoriteKey] = favorite } // read locks - if _, ok := mdKeysMap[LockdiscoveryKey]; returnAllKeys || ok { + // FIXME move to fieldmask + if _, ok := mdKeysMap[LockdiscoveryKey]; returnAllMetadata || ok { if n.hasLocks(ctx) { err = readLocksIntoOpaque(ctx, n, ri) if err != nil { @@ -718,21 +733,36 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi } // share indicator - if _, ok := mdKeysMap[ShareTypesKey]; returnAllKeys || ok { - if n.hasUserShares(ctx) { - metadata[ShareTypesKey] = UserShareType + 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()) } } // checksums - if _, ok := mdKeysMap[ChecksumsKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_FILE) && (returnAllKeys || ok) { + // FIXME move to fieldmask + if _, ok := mdKeysMap[ChecksumsKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_FILE) && (returnAllMetadata || 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 - if _, ok := mdKeysMap[QuotaKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_CONTAINER) && returnAllKeys || ok { + // FIXME move to fieldmask + if _, ok := mdKeysMap[QuotaKey]; (nodeType == provider.ResourceType_RESOURCE_TYPE_CONTAINER) && returnAllMetadata || ok { if n.SpaceRoot != nil && n.SpaceRoot.InternalPath() != "" { readQuotaIntoOpaque(ctx, n.SpaceRoot.InternalPath(), ri) } @@ -750,7 +780,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]; returnAllKeys || ok { + if _, ok := mdKeysMap[k]; returnAllMetadata || ok { if val, err := xattrs.Get(nodePath, attrs[i]); err == nil { metadata[k] = val } else { @@ -784,7 +814,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 fount") + appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not found") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") } @@ -806,7 +836,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 fount") + appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not found") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") } @@ -1077,19 +1107,28 @@ func ReadBlobIDAttr(path string) (string, error) { return attr, nil } -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 +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) } } - return false + return types } 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 cfae71aadb..9b6a460421 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{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []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{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []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{}, false) + ri, err = n.AsResourceInfo(env.Ctx, &perms, []string{}, []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{}, false) + ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{}, []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 618f23f274..8dc69a8189 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -181,13 +181,6 @@ 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)) @@ -207,7 +200,7 @@ func (p *Permissions) HasPermission(ctx context.Context, n *Node, check func(*pr } } - // also check permissions on root, eg. for for project spaces + // also check permissions on root, eg. 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 4b85656d23..7112e3fe74 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{}, false) + riBefore, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, []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{}, false) + riAfter, err := dir.AsResourceInfo(env.Ctx, &perms, []string{}, []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 0d13917e57..5f4276ed5b 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{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}, []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{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}, []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{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}, []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{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}, []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{}) + resources, err := fs.ListFolder(ctx, rootRef, []string{}, []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 00bd59d232..c0f7c45e72 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) (*provider.ResourceInfo, error) { +func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []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 []string) ([]*provider.ResourceInfo, error) { +func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []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) + md, err := fs.GetMD(ctx, ref, nil, 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) + md, err := fs.GetMD(ctx, ref, nil, 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) + md, err := fs.GetMD(ctx, ref, nil, 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) + md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil, 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) + md, err := fs.GetMD(ctx, &provider.Reference{Path: ref.Path}, nil, nil) if err != nil { return err } diff --git a/pkg/storage/utils/eosfs/spaces.go b/pkg/storage/utils/eosfs/spaces.go index 477265eb15..78d1cc20a0 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{}) + info, err := fs.GetMD(ctx, &provider.Reference{Path: relPath}, []string{}, nil) 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 452432b276..a013d7d0ba 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) (*provider.ResourceInfo, error) { +func (fs *localfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []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 []string) ([]*provider.ResourceInfo, error) { +func (fs *localfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []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 aa77ab3f5d..1f361564c9 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -1289,16 +1289,6 @@ _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) @@ -1407,9 +1397,6 @@ 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 408f81139e..6b53ac9c6b 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -1307,16 +1307,6 @@ _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) @@ -1410,9 +1400,6 @@ _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)