diff --git a/changelog/unreleased/disallow-moves-between-shares.md b/changelog/unreleased/disallow-moves-between-shares.md new file mode 100644 index 0000000000..5c453e83f1 --- /dev/null +++ b/changelog/unreleased/disallow-moves-between-shares.md @@ -0,0 +1,5 @@ +Bugfix: Do not allow moves between shares + +We no longer allow moves between shares, even if they resolve to the same space. + +https://github.com/cs3org/reva/pull/4318 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index e363aec269..207992f515 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -643,9 +643,10 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide Status: rpcStatus, }, nil } - if srcReceivedShare.Share.ResourceId.SpaceId != dstReceivedShare.Share.ResourceId.SpaceId { + + if dstReceivedShare.Share.Id.OpaqueId != srcReceivedShare.Share.Id.OpaqueId { return &provider.MoveResponse{ - Status: status.NewInvalid(ctx, "sharesstorageprovider: can not move between shares on different storages"), + Status: status.NewUnimplemented(ctx, nil, "sharesstorageprovider: can not move between shares"), }, nil } diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 98db834907..eac7ac8035 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -89,8 +89,8 @@ var _ = Describe("Sharesstorageprovider", func() { OpaqueId: "shareid", }, ResourceId: &sprovider.ResourceId{ - StorageId: "share1-storageid", - SpaceId: "share1-storageid", + StorageId: "storageid", + SpaceId: "storageid", OpaqueId: "shareddir", }, Permissions: &collaboration.SharePermissions{ @@ -102,7 +102,7 @@ var _ = Describe("Sharesstorageprovider", func() { }, }, MountPoint: &sprovider.Reference{ - Path: "oldname", + Path: "share1-shareddir", }, } @@ -110,12 +110,12 @@ var _ = Describe("Sharesstorageprovider", func() { State: collaboration.ShareState_SHARE_STATE_ACCEPTED, Share: &collaboration.Share{ Id: &collaboration.ShareId{ - OpaqueId: "shareidtwo", + OpaqueId: "shareid2", }, ResourceId: &sprovider.ResourceId{ - StorageId: "share1-storageid", - SpaceId: "share1-storageid", - OpaqueId: "shareddir", + StorageId: "storageid", + SpaceId: "storageid", + OpaqueId: "shareddir2", }, Permissions: &collaboration.SharePermissions{ Permissions: &sprovider.ResourcePermissions{ @@ -126,7 +126,7 @@ var _ = Describe("Sharesstorageprovider", func() { }, }, MountPoint: &sprovider.Reference{ - Path: "share1-shareddir", + Path: "share2-shareddir2", }, } @@ -298,6 +298,12 @@ var _ = Describe("Sharesstorageprovider", func() { }) } return resp + case utils.ResourceIDEqual(req.Ref.ResourceId, BaseShareTwo.Share.ResourceId): + resp := &sprovider.ListContainerResponse{ + Status: status.NewOK(context.Background()), + Infos: []*sprovider.ResourceInfo{}, + } + return resp case utils.ResourceIDEqual(req.Ref.ResourceId, BaseShareTwo.Share.ResourceId): return &sprovider.ListContainerResponse{ Status: status.NewOK(context.Background()), @@ -772,11 +778,11 @@ var _ = Describe("Sharesstorageprovider", func() { req := &sprovider.MoveRequest{ Source: &sprovider.Reference{ ResourceId: ShareJail, - Path: "./oldname", + Path: "./share1-shareddir", }, Destination: &sprovider.Reference{ ResourceId: ShareJail, - Path: "./newname", + Path: "./share1-shareddir-renamed", }, } res, err := s.Move(ctx, req) @@ -790,17 +796,37 @@ var _ = Describe("Sharesstorageprovider", func() { It("refuses to move a file between shares", func() { req := &sprovider.MoveRequest{ Source: &sprovider.Reference{ - Path: "/shares/share1-shareddir/share1-shareddir-file", + ResourceId: ShareJail, + Path: "./share1-shareddir/share1-shareddir-file", }, Destination: &sprovider.Reference{ - Path: "/shares/share2-shareddir/share2-shareddir-file", + ResourceId: ShareJail, + Path: "./share2-shareddir2/share1-shareddir-file", }, } res, err := s.Move(ctx, req) gatewayClient.AssertNotCalled(GinkgoT(), "Move", mock.Anything, mock.Anything) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) - Expect(res.Status.Code).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_UNIMPLEMENTED)) + }) + + It("refuses to move a file between shares resolving to the same space", func() { + req := &sprovider.MoveRequest{ + Source: &sprovider.Reference{ + ResourceId: ShareJail, + Path: "./share1-shareddir/share1-shareddir-file", + }, + Destination: &sprovider.Reference{ + ResourceId: ShareJail, + Path: "./share2-shareddir2/share1-shareddir-file", + }, + } + res, err := s.Move(ctx, req) + gatewayClient.AssertNotCalled(GinkgoT(), "Move", mock.Anything, mock.Anything) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_UNIMPLEMENTED)) }) It("moves a file", func() {