From be8954f2a0648f20ab8758a18d5955b0ab3c3ba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 31 May 2022 11:12:46 +0000 Subject: [PATCH] fix spaces share jail usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/share-jail-fixes.md | 5 + .../sharesstorageprovider.go | 214 +++++++++--------- .../sharesstorageprovider_test.go | 28 ++- internal/http/services/owncloud/ocdav/move.go | 9 +- .../expected-failures-on-OCIS-storage.md | 4 - 5 files changed, 144 insertions(+), 116 deletions(-) create mode 100644 changelog/unreleased/share-jail-fixes.md diff --git a/changelog/unreleased/share-jail-fixes.md b/changelog/unreleased/share-jail-fixes.md new file mode 100644 index 0000000000..693403eab2 --- /dev/null +++ b/changelog/unreleased/share-jail-fixes.md @@ -0,0 +1,5 @@ +Bugfix: Share jail now works properly when accessed as a space + +When accessing shares via the virtual share jail we now build correct relative references before forwarding the requests to the correct storage provider. + +https://github.com/cs3org/reva/pull/2904 \ No newline at end of file diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 1342ff46bf..713f260698 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -109,7 +109,7 @@ func (s *service) SetArbitraryMetadata(ctx context.Context, req *provider.SetArb _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -117,17 +117,15 @@ func (s *service) SetArbitraryMetadata(ctx context.Context, req *provider.SetArb if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.SetArbitraryMetadataResponse{ Status: rpcStatus, }, nil } return s.gateway.SetArbitraryMetadata(ctx, &provider.SetArbitraryMetadataRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), ArbitraryMetadata: req.ArbitraryMetadata, }) } @@ -137,7 +135,7 @@ func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.Unse _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -145,17 +143,15 @@ func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.Unse if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.UnsetArbitraryMetadataResponse{ Status: rpcStatus, }, nil } return s.gateway.UnsetArbitraryMetadata(ctx, &provider.UnsetArbitraryMetadataRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, }) } @@ -165,7 +161,7 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -173,17 +169,15 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.InitiateFileDownloadResponse{ Status: rpcStatus, }, nil } + gwres, err := s.gateway.InitiateFileDownload(ctx, &provider.InitiateFileDownloadRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), }) if err != nil { return nil, err @@ -221,7 +215,7 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -229,17 +223,14 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.InitiateFileUploadResponse{ Status: rpcStatus, }, nil } gwres, err := s.gateway.InitiateFileUpload(ctx, &provider.InitiateFileUploadRequest{ - Opaque: req.Opaque, - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), Options: req.Options, }) if err != nil { @@ -458,6 +449,9 @@ 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 { + // we could not stat the share, skip it + continue } // add the resourceID for the grant if receivedShare.Share.ResourceId != nil { @@ -504,7 +498,7 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -512,17 +506,15 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.CreateContainerResponse{ Status: rpcStatus, }, nil } return s.gateway.CreateContainer(ctx, &provider.CreateContainerRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), }) } @@ -531,7 +523,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -540,7 +532,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.DeleteResponse{ Status: rpcStatus, }, nil @@ -560,10 +552,8 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro } return s.gateway.Delete(ctx, &provider.DeleteRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), }) } @@ -585,24 +575,23 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide // - the registry needs to invalidate the alias // - the rhe share manager needs to change the name // ... but which storageprovider will receive the move request??? - srcReceivedShare, rpcStatus, err := s.resolveReference(ctx, req.Source) + srcReceivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Source) if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.MoveResponse{ Status: rpcStatus, }, nil } - // can we do a rename - if utils.ResourceIDEqual(req.Source.ResourceId, req.Destination.ResourceId) && - // only if we are responsible for the space - req.Source.ResourceId.StorageId == utils.ShareStorageProviderID && - // only if the source path has no path segment - req.Source.Path == "." && - // only if the destination is a dot followed by a single path segment, e.g. './new' - len(strings.SplitN(req.Destination.Path, "/", 3)) == 2 { + // we can do a rename + // if the source is a share jail child where the path is . + if ((isShareJailChild(req.Source.ResourceId) && req.Source.Path == ".") || + // or if the source is the share jail with a single path segment, e.g. './old' + (isShareJailRoot(req.Source.ResourceId) && len(strings.SplitN(req.Destination.Path, "/", 3)) == 2)) && + // and if the destination is a dot followed by a single path segment, e.g. './new' + isShareJailRoot(req.Destination.ResourceId) && len(strings.SplitN(req.Destination.Path, "/", 3)) == 2 { // Change the MountPoint of the share, it has no relative prefix srcReceivedShare.MountPoint = &provider.Reference{ @@ -624,11 +613,11 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide }, nil } - dstReceivedShare, rpcStatus, err2 := s.resolveReference(ctx, req.Destination) + dstReceivedShare, rpcStatus, err2 := s.resolveAcceptedShare(ctx, req.Destination) if err2 != nil { return nil, err2 } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.MoveResponse{ Status: rpcStatus, }, nil @@ -640,17 +629,22 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide } return s.gateway.Move(ctx, &provider.MoveRequest{ - Source: &provider.Reference{ - ResourceId: srcReceivedShare.Share.ResourceId, - Path: req.Source.Path, - }, - Destination: &provider.Reference{ - ResourceId: dstReceivedShare.Share.ResourceId, - Path: req.Destination.Path, - }, + Opaque: req.Opaque, + Source: buildReferenceInShare(req.Source, srcReceivedShare), + Destination: buildReferenceInShare(req.Destination, dstReceivedShare), }) } +func isShareJailRoot(id *provider.ResourceId) bool { + _, space := storagespace.SplitStorageID(id.StorageId) + return space == utils.ShareStorageProviderID && id.OpaqueId == utils.ShareStorageProviderID +} + +func isShareJailChild(id *provider.ResourceId) bool { + _, space := storagespace.SplitStorageID(id.StorageId) + return space == utils.ShareStorageProviderID && id.OpaqueId != utils.ShareStorageProviderID +} + // SetLock puts a lock on the given reference func (s *service) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provider.SetLockResponse, error) { return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") @@ -676,7 +670,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - if isVirtualRoot(req.Ref.ResourceId) && (req.Ref.Path == "" || req.Ref.Path == ".") { + if isVirtualRoot(req.Ref) { receivedShares, shareMd, err := s.fetchShares(ctx) if err != nil { return nil, err @@ -709,7 +703,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide }, }, nil } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -718,7 +712,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.StatResponse{ Status: rpcStatus, }, nil @@ -730,40 +724,30 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide }, nil } - path := req.Ref.Path - if receivedShare.MountPoint.Path == strings.TrimPrefix(req.Ref.Path, "./") { - path = "." - } - // TODO return reference? return s.gateway.Stat(ctx, &provider.StatRequest{ - Opaque: req.Opaque, - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, }) + + // we need to rewrite the id if the request was made relative to the virtual space root? + // but won't that be problematic for eg. wopi because it needs the correct id? + // the web ui seems to continue navigating based on the id. + } func (s *service) ListContainerStream(req *provider.ListContainerStreamRequest, ss provider.ProviderAPI_ListContainerStreamServer) error { return gstatus.Errorf(codes.Unimplemented, "method not implemented") } - -func isVirtualRoot(id *provider.ResourceId) bool { - return utils.ResourceIDEqual(id, &provider.ResourceId{ - StorageId: utils.ShareStorageProviderID, - OpaqueId: utils.ShareStorageProviderID, - }) -} func (s *service) ListContainer(ctx context.Context, req *provider.ListContainerRequest) (*provider.ListContainerResponse, error) { if req.Ref.GetResourceId() != nil { _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - if isVirtualRoot(req.Ref.ResourceId) { + if isVirtualRoot(req.Ref) { // The root is empty, it is filled by mountpoints - // but when accessing the root via /dav/spaces we need to list the content + // so, when accessing the root via /dav/spaces, we need to list the accepted shares with their mountpoint receivedShares, _, err := s.fetchShares(ctx) if err != nil { @@ -814,7 +798,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer Infos: infos, }, nil } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -823,18 +807,15 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.ListContainerResponse{ Status: rpcStatus, }, nil } return s.gateway.ListContainer(ctx, &provider.ListContainerRequest{ - Opaque: req.Opaque, - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), ArbitraryMetadataKeys: req.ArbitraryMetadataKeys, }) } @@ -843,7 +824,7 @@ func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVe _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -852,17 +833,15 @@ func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVe if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.ListFileVersionsResponse{ Status: rpcStatus, }, nil } return s.gateway.ListFileVersions(ctx, &provider.ListFileVersionsRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), }) } @@ -871,7 +850,7 @@ func (s *service) RestoreFileVersion(ctx context.Context, req *provider.RestoreF _, req.Ref.ResourceId.StorageId = storagespace.SplitStorageID(req.Ref.ResourceId.StorageId) } - receivedShare, rpcStatus, err := s.resolveReference(ctx, req.Ref) + receivedShare, rpcStatus, err := s.resolveAcceptedShare(ctx, req.Ref) appctx.GetLogger(ctx).Debug(). Interface("ref", req.Ref). Interface("received_share", receivedShare). @@ -880,17 +859,15 @@ func (s *service) RestoreFileVersion(ctx context.Context, req *provider.RestoreF if err != nil { return nil, err } - if rpcStatus != nil { + if rpcStatus.Code != rpc.Code_CODE_OK { return &provider.RestoreFileVersionResponse{ Status: rpcStatus, }, nil } return s.gateway.RestoreFileVersion(ctx, &provider.RestoreFileVersionRequest{ - Ref: &provider.Reference{ - ResourceId: receivedShare.Share.ResourceId, - Path: req.Ref.Path, - }, + Opaque: req.Opaque, + Ref: buildReferenceInShare(req.Ref, receivedShare), }) } @@ -954,7 +931,7 @@ func (s *service) GetQuota(ctx context.Context, req *provider.GetQuotaRequest) ( }, nil } -func (s *service) resolveReference(ctx context.Context, ref *provider.Reference) (*collaboration.ReceivedShare, *rpc.Status, error) { +func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Reference) (*collaboration.ReceivedShare, *rpc.Status, error) { // treat absolute id based references as relative ones if ref.Path == "" { ref.Path = "." @@ -965,13 +942,13 @@ func (s *service) resolveReference(ctx context.Context, ref *provider.Reference) } // look up share for this resourceid lsRes, err := s.sharesProviderClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ - // FIXME filter by received shares for resource id - listing all shares is tooo expensive! + // FIXME filter by received shares for reference - listing all shares is tooo expensive! }) if err != nil { return nil, nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } if lsRes.Status.Code != rpc.Code_CODE_OK { - return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest") + return nil, lsRes.Status, nil } for _, receivedShare := range lsRes.Shares { if receivedShare.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { @@ -985,10 +962,16 @@ func (s *service) resolveReference(ctx context.Context, ref *provider.Reference) switch { case utils.ResourceIDEqual(ref.ResourceId, root): // we have a virtual node - return receivedShare, nil, nil + return receivedShare, lsRes.Status, nil case utils.ResourceIDEqual(ref.ResourceId, receivedShare.Share.ResourceId): // we have a mount point - return receivedShare, nil, nil + return receivedShare, lsRes.Status, nil + case utils.ResourceIDEqual(ref.ResourceId, &provider.ResourceId{ + StorageId: utils.ShareStorageProviderID, + OpaqueId: utils.ShareStorageProviderID, + }) && strings.HasPrefix(strings.TrimPrefix(ref.Path, "./"), receivedShare.MountPoint.Path): + // we have a mount point referenced by the share jail id + return receivedShare, lsRes.Status, nil default: continue } @@ -1031,7 +1014,7 @@ func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedSha if rs.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { continue } - sRes, err := s.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: rs.Share.ResourceId}}) + sRes, err := s.gateway.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: rs.Share.ResourceId}}) if err != nil { appctx.GetLogger(ctx).Error(). Err(err). @@ -1085,3 +1068,26 @@ func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd ma } return earliestShare, atLeastOneAccepted } + +func buildReferenceInShare(ref *provider.Reference, s *collaboration.ReceivedShare) *provider.Reference { + path := ref.Path + if isVirtualRootResourceID(ref.ResourceId) { + // we need to cut off the mountpoint from the path in the request reference + path = utils.MakeRelativePath(strings.TrimPrefix(strings.TrimPrefix(path, "./"), s.MountPoint.Path)) + } + return &provider.Reference{ + ResourceId: s.Share.ResourceId, + Path: path, + } +} + +func isVirtualRoot(ref *provider.Reference) bool { + return isVirtualRootResourceID(ref.ResourceId) && (ref.Path == "" || ref.Path == "." || ref.Path == "./") +} + +func isVirtualRootResourceID(id *provider.ResourceId) bool { + return utils.ResourceIDEqual(id, &provider.ResourceId{ + StorageId: utils.ShareStorageProviderID, + OpaqueId: utils.ShareStorageProviderID, + }) +} diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 8e1c28ead9..ba301c1389 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -43,6 +43,11 @@ import ( ) var ( + ShareJail = &sprovider.ResourceId{ + StorageId: utils.ShareStorageProviderID, + OpaqueId: utils.ShareStorageProviderID, + } + BaseShare = &collaboration.ReceivedShare{ State: collaboration.ShareState_SHARE_STATE_ACCEPTED, Share: &collaboration.Share{ @@ -561,7 +566,28 @@ var _ = Describe("Sharesstorageprovider", func() { Path: ".", }, Destination: &sprovider.Reference{ - ResourceId: BaseShare.Share.ResourceId, + ResourceId: ShareJail, + Path: "./newname", + }, + } + res, err := s.Move(ctx, req) + gw.AssertNotCalled(GinkgoT(), "Move", mock.Anything, mock.Anything) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_OK)) + sharesProviderClient.AssertCalled(GinkgoT(), "UpdateReceivedShare", mock.Anything, mock.Anything) + }) + + It("renames a sharejail entry", func() { + sharesProviderClient.On("UpdateReceivedShare", mock.Anything, mock.Anything).Return(nil, nil) + + req := &sprovider.MoveRequest{ + Source: &sprovider.Reference{ + ResourceId: ShareJail, + Path: "./oldname", + }, + Destination: &sprovider.Reference{ + ResourceId: ShareJail, Path: "./newname", }, } diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 84a2cfae32..842b99b785 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -69,7 +69,7 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string) srcSpace, status, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, srcPath) if err != nil { - sublog.Error().Err(err).Str("path", srcPath).Msg("failed to look up storage space") + sublog.Error().Err(err).Str("path", srcPath).Msg("failed to look up source storage space") w.WriteHeader(http.StatusInternalServerError) return } @@ -79,7 +79,7 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string) } dstSpace, status, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, dstPath) if err != nil { - sublog.Error().Err(err).Str("path", srcPath).Msg("failed to look up storage space") + sublog.Error().Err(err).Str("path", dstPath).Msg("failed to look up destination storage space") w.WriteHeader(http.StatusInternalServerError) return } @@ -88,11 +88,6 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string) return } - // FIXME I suck - if dstSpace.Root.OpaqueId == utils.ShareStorageProviderID { - dstSpace.Root = srcSpace.Root - } - s.handleMove(ctx, w, r, spacelookup.MakeRelativeReference(srcSpace, srcPath, false), spacelookup.MakeRelativeReference(dstSpace, dstPath, false), sublog) } diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index 0b5f14261b..348f10a390 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -1339,10 +1339,6 @@ _The below features have been added after I last categorized them. AFAICT they a - [apiWebdavUploadTUS/optionsRequest.feature:85](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/optionsRequest.feature#L85) - [apiWebdavUploadTUS/optionsRequest.feature:100](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/optionsRequest.feature#L100) -#### [Share inaccessible if folder with same name was deleted and recreated](https://github.com/owncloud/ocis/issues/1787) -- [apiShareReshareToShares1/reShare.feature:259](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareReshareToShares1/reShare.feature#L259) -- [apiShareReshareToShares1/reShare.feature:260](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareReshareToShares1/reShare.feature#L260) - #### [incorrect ocs(v2) status value when getting info of share that does not exist should be 404, gives 998](https://github.com/owncloud/product/issues/250) _ocs: api compatibility, return correct status code_ - [apiShareOperationsToShares2/shareAccessByID.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares2/shareAccessByID.feature#L48)