Skip to content

Commit

Permalink
Merge pull request #4284 from aduffeck/disallow-moves
Browse files Browse the repository at this point in the history
Disallow moves
  • Loading branch information
aduffeck authored Oct 27, 2023
2 parents 04eb8f6 + fe4384d commit a0a3e4f
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 15 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/disallow-moves-between-shares.md
Original file line number Diff line number Diff line change
@@ -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/4284
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -102,20 +102,20 @@ var _ = Describe("Sharesstorageprovider", func() {
},
},
MountPoint: &sprovider.Reference{
Path: "oldname",
Path: "share1-shareddir",
},
}

BaseShareTwo = &collaboration.ReceivedShare{
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{
Expand All @@ -126,7 +126,7 @@ var _ = Describe("Sharesstorageprovider", func() {
},
},
MountPoint: &sprovider.Reference{
Path: "share1-shareddir",
Path: "share2-shareddir2",
},
}

Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down

0 comments on commit a0a3e4f

Please sign in to comment.