Skip to content

Commit

Permalink
Merge pull request #4571 from dragonchaser/ocis-1230-do-not-overwrite…
Browse files Browse the repository at this point in the history
…-parent

add parentcheck
  • Loading branch information
dragonchaser committed Mar 14, 2024
2 parents 352a246 + 05bbbd1 commit 1aca43c
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 8 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/check-parent-on-copy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Prevent copying a file to a parent folder

When copying a file to a parent folder, the file would be copied to the parent folder, but the file would not be removed from the original folder.

https://github.com/cs3org/reva/pull/4571
https://github.com/owncloud/ocis/issues/1230
27 changes: 27 additions & 0 deletions internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,33 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
return nil
}

isParent, err := s.referenceIsChildOf(ctx, s.gatewaySelector, srcRef, dstRef)
if err != nil {
switch err.(type) {
case errtypes.IsNotSupported:
log.Error().Err(err).Msg("can not detect recursive copy operation. missing machine auth configuration?")
w.WriteHeader(http.StatusForbidden)
default:
log.Error().Err(err).Msg("error while trying to detect recursive copy operation")
w.WriteHeader(http.StatusInternalServerError)
}
}

if isParent {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into its parent", "")
errors.HandleWebdavError(log, w, b, err)
return nil

}

if srcRef.Path == dstRef.Path {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "source and destination are the same", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}

oh := r.Header.Get(net.HeaderOverwrite)
overwrite, err := net.ParseOverwrite(oh)
if err != nil {
Expand Down
22 changes: 22 additions & 0 deletions internal/http/services/owncloud/ocdav/move.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,28 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
return
}

isParent, err := s.referenceIsChildOf(ctx, s.gatewaySelector, src, dst)
if err != nil {
switch err.(type) {
case errtypes.IsNotFound:
w.WriteHeader(http.StatusNotFound)
case errtypes.IsNotSupported:
log.Error().Err(err).Msg("can not detect recursive move operation. missing machine auth configuration?")
w.WriteHeader(http.StatusForbidden)
default:
log.Error().Err(err).Msg("error while trying to detect recursive move operation")
w.WriteHeader(http.StatusInternalServerError)
}
return
}
if isParent {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into its parent", "")
errors.HandleWebdavError(&log, w, b, err)
return

}

oh := r.Header.Get(net.HeaderOverwrite)
log.Debug().Str("overwrite", oh).Msg("move")

Expand Down
23 changes: 15 additions & 8 deletions internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/cs3org/reva/v2/tests/cs3mocks/mocks"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -958,14 +959,20 @@ var _ = Describe("ocdav", func() {
req.Header.Set(net.HeaderDestination, basePath+dstPath)
req.Header.Set("Overwrite", "T")

client.On("GetPath", mock.Anything, mock.Anything).Return(&cs3storageprovider.GetPathResponse{
Status: status.NewOK(ctx),
Path: "/file",
}, nil).Once()
client.On("GetPath", mock.Anything, mock.Anything).Return(&cs3storageprovider.GetPathResponse{
Status: status.NewOK(ctx),
Path: "/dstFileName",
}, nil).Once()
client.On("GetPath", mock.Anything, mock.Anything).Return(func(ctx context.Context, req *cs3storageprovider.GetPathRequest, _ ...grpc.CallOption) (*cs3storageprovider.GetPathResponse, error) {
switch req.ResourceId.OpaqueId {
case "dstId":
return &cs3storageprovider.GetPathResponse{
Status: status.NewOK(ctx),
Path: "/dstFileName",
}, nil
default:
return &cs3storageprovider.GetPathResponse{
Status: status.NewOK(ctx),
Path: "/file",
}, nil
}
})

client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool {
return req.Ref.Path == mReq.Source.Path
Expand Down

0 comments on commit 1aca43c

Please sign in to comment.