Skip to content

Commit

Permalink
Revert "Prevent recursive copy/move operations on the API level (cs3o…
Browse files Browse the repository at this point in the history
…rg#3009)"

This reverts commit fd7ba90.
  • Loading branch information
kobergj committed Jul 14, 2022
1 parent 5145317 commit b0ea559
Show file tree
Hide file tree
Showing 13 changed files with 14 additions and 194 deletions.
5 changes: 0 additions & 5 deletions changelog/unreleased/prevent-recursive-copy-move.md

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/cs3org/reva/v2/pkg/appctx"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/errtypes"
"github.com/cs3org/reva/v2/pkg/rgrpc"
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
Expand Down Expand Up @@ -281,14 +280,6 @@ func (s *service) GetPath(ctx context.Context, req *provider.GetPathRequest) (*p
// - getPath of every received share on the same space - needs also owner permissions -> needs machine auth
// - find the shortest root path that is a prefix of the resource path
// alternatively implement this on storageprovider - it needs to know about grants to do so

if isShareJailRoot(req.ResourceId) {
return &provider.GetPathResponse{
Status: status.NewOK(ctx),
Path: "/",
}, nil
}

return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented")
}

Expand Down Expand Up @@ -679,10 +670,6 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide
}

if isVirtualRoot(req.Ref) {
owner, ok := ctxpkg.ContextGetUser(ctx)
if !ok {
return nil, fmt.Errorf("missing user in context")
}
receivedShares, shareMd, err := s.fetchShares(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -711,8 +698,7 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide
PermissionSet: &provider.ResourcePermissions{
// TODO
},
Etag: shareMd[earliestShare.Id.OpaqueId].ETag,
Owner: owner.Id,
Etag: shareMd[earliestShare.Id.OpaqueId].ETag,
},
}, nil
}
Expand Down
33 changes: 7 additions & 26 deletions internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
"github.com/cs3org/reva/v2/pkg/appctx"
"github.com/cs3org/reva/v2/pkg/errtypes"
"github.com/cs3org/reva/v2/pkg/rhttp"
"github.com/cs3org/reva/v2/pkg/rhttp/router"
"github.com/cs3org/reva/v2/pkg/utils"
Expand Down Expand Up @@ -487,31 +486,6 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, clie
}

func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger) *copy {
client, err := s.getClient()
if err != nil {
log.Error().Err(err).Msg("error getting grpc client")
w.WriteHeader(http.StatusInternalServerError)
return nil
}

isChild, err := s.referenceIsChildOf(ctx, client, dstRef, srcRef)
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 isChild {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into one of its children", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}

oh := r.Header.Get(net.HeaderOverwrite)
overwrite, err := net.ParseOverwrite(oh)
if err != nil {
Expand Down Expand Up @@ -539,6 +513,13 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re

log.Debug().Bool("overwrite", overwrite).Str("depth", depth.String()).Msg("copy")

client, err := s.getClient()
if err != nil {
log.Error().Err(err).Msg("error getting grpc client")
w.WriteHeader(http.StatusInternalServerError)
return nil
}

srcStatReq := &provider.StatRequest{Ref: srcRef}
srcStatRes, err := client.Stat(ctx, srcStatReq)
if err != nil {
Expand Down
31 changes: 6 additions & 25 deletions internal/http/services/owncloud/ocdav/move.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
"github.com/cs3org/reva/v2/pkg/appctx"
"github.com/cs3org/reva/v2/pkg/errtypes"
"github.com/cs3org/reva/v2/pkg/rhttp/router"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
Expand Down Expand Up @@ -133,37 +132,19 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI
}

func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Request, src, dst *provider.Reference, log zerolog.Logger) {
client, err := s.getClient()
if err != nil {
log.Error().Err(err).Msg("error getting grpc client")
w.WriteHeader(http.StatusInternalServerError)
return
}
oh := r.Header.Get(net.HeaderOverwrite)
log.Debug().Str("overwrite", oh).Msg("move")

isChild, err := s.referenceIsChildOf(ctx, client, dst, src)
overwrite, err := net.ParseOverwrite(oh)
if err != nil {
switch err.(type) {
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)
}
}
if isChild {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into one of its children", "")
errors.HandleWebdavError(&log, w, b, err)
return
}

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

overwrite, err := net.ParseOverwrite(oh)
client, err := s.getClient()
if err != nil {
w.WriteHeader(http.StatusBadRequest)
log.Error().Err(err).Msg("error getting grpc client")
w.WriteHeader(http.StatusInternalServerError)
return
}

Expand Down
109 changes: 0 additions & 109 deletions internal/http/services/owncloud/ocdav/ocdav.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ import (

"github.com/ReneKroon/ttlcache/v2"
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
"github.com/cs3org/reva/v2/pkg/appctx"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
Expand All @@ -43,15 +41,10 @@ import (
"github.com/cs3org/reva/v2/pkg/storage/favorite"
"github.com/cs3org/reva/v2/pkg/storage/favorite/registry"
"github.com/cs3org/reva/v2/pkg/storage/utils/templates"
"github.com/cs3org/reva/v2/pkg/storagespace"
rtrace "github.com/cs3org/reva/v2/pkg/trace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/mitchellh/mapstructure"
"github.com/rs/zerolog"
"go.opentelemetry.io/otel/trace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"
)

// name is the Tracer name used to identify this instrumentation library.
Expand Down Expand Up @@ -114,8 +107,6 @@ type Config struct {
Product string `mapstructure:"product"`
ProductName string `mapstructure:"product_name"`
ProductVersion string `mapstructure:"product_version"`

MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"`
}

func (c *Config) init() {
Expand Down Expand Up @@ -393,103 +384,3 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
headers.Set("Strict-Transport-Security", "max-age=63072000")
}
}

func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) {
if machineAuthAPIKey == "" {
return nil, errtypes.NotSupported("machine auth not configured")
}
// Get auth
granteeCtx := ctxpkg.ContextSetUser(context.Background(), &userpb.User{Id: userID})

authRes, err := client.Authenticate(granteeCtx, &gateway.AuthenticateRequest{
Type: "machine",
ClientId: "userid:" + userID.OpaqueId,
ClientSecret: machineAuthAPIKey,
})
if err != nil {
return nil, err
}
if authRes.GetStatus().GetCode() != rpc.Code_CODE_OK {
return nil, errtypes.NewErrtypeFromStatus(authRes.Status)
}
granteeCtx = metadata.AppendToOutgoingContext(granteeCtx, ctxpkg.TokenHeader, authRes.Token)
return granteeCtx, nil
}

func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
parentStatRes, err := client.Stat(ctx, &provider.StatRequest{Ref: parent})
if err != nil {
return false, err
}
parentAuthCtx, err := authContextForUser(client, parentStatRes.Info.Owner, s.c.MachineAuthAPIKey)
if err != nil {
return false, err
}
parentPathRes, err := client.GetPath(parentAuthCtx, &provider.GetPathRequest{ResourceId: parentStatRes.Info.Id})
if err != nil {
return false, err
}

childStatRes, err := client.Stat(ctx, &provider.StatRequest{Ref: child})
if err != nil {
return false, err
}
if childStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND && utils.IsRelativeReference(child) && child.Path != "." {
childParentRef := &provider.Reference{
ResourceId: child.ResourceId,
Path: utils.MakeRelativePath(path.Dir(child.Path)),
}
childStatRes, err = client.Stat(ctx, &provider.StatRequest{Ref: childParentRef})
if err != nil {
return false, err
}
}
childAuthCtx, err := authContextForUser(client, childStatRes.Info.Owner, s.c.MachineAuthAPIKey)
if err != nil {
return false, err
}
childPathRes, err := client.GetPath(childAuthCtx, &provider.GetPathRequest{ResourceId: childStatRes.Info.Id})
if err != nil {
return false, err
}

cp := childPathRes.Path + "/"
pp := parentPathRes.Path + "/"
return strings.HasPrefix(cp, pp), nil
}

func (s *svc) referenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
_, csid := storagespace.SplitStorageID(child.ResourceId.StorageId)
_, psid := storagespace.SplitStorageID(parent.ResourceId.StorageId)

if child.ResourceId.StorageId != parent.ResourceId.StorageId {
return false, nil // Not on the same storage -> not a child
}

if utils.ResourceIDEqual(child.ResourceId, parent.ResourceId) {
return strings.HasPrefix(child.Path, parent.Path+"/"), nil // Relative to the same resource -> compare paths
}

if csid == utils.ShareStorageProviderID || psid == utils.ShareStorageProviderID {
// the sharesstorageprovider needs some special handling
return s.sspReferenceIsChildOf(ctx, client, child, parent)
}

// the references are on the same storage but relative to different resources
// -> we need to get the path for both resources
childPathRes, err := client.GetPath(ctx, &provider.GetPathRequest{ResourceId: child.ResourceId})
if err != nil {
if st, ok := status.FromError(err); ok && st.Code() == codes.Unimplemented {
return false, nil // the storage provider doesn't support GetPath() -> rely on it taking care of recursion issues
}
return false, err
}
parentPathRes, err := client.GetPath(ctx, &provider.GetPathRequest{ResourceId: parent.ResourceId})
if err != nil {
return false, err
}

cp := path.Join(childPathRes.Path, child.Path) + "/"
pp := path.Join(parentPathRes.Path, parent.Path) + "/"
return strings.HasPrefix(cp, pp), nil
}
7 changes: 0 additions & 7 deletions pkg/micro/ocdav/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,6 @@ func JWTSecret(s string) Option {
}
}

// MachineAuthAPIKey provides a function to set the machine auth api key option.
func MachineAuthAPIKey(s string) Option {
return func(o *Options) {
o.config.MachineAuthAPIKey = s
}
}

// Context provides a function to set the context option.
func Context(val context.Context) Option {
return func(o *Options) {
Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/drone/frontend-global.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ files_namespace = "/"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/"
machine_auth_apikey = "change-me-please"

[http.services.ocs]

Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/drone/frontend.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ files_namespace = "/users/{{.Id.OpaqueId}}"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/users/{{.Id.OpaqueId}}"
machine_auth_apikey = "change-me-please"

[http.services.ocs]
machine_auth_apikey = "change-me-please"
Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/local-mesh/frontend-global.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ files_namespace = "/"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/"
machine_auth_apikey = "change-me-please"

[http.services.ocs]

Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/local-mesh/frontend.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ files_namespace = "/users"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/home"
machine_auth_apikey = "change-me-please"

# serve /ocs which contains the sharing and user provisioning api of owncloud classic
[http.services.ocs]
Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/local/combined.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ files_namespace = "/personal/{{.Id.OpaqueId}}"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/home"
machine_auth_apikey = "change-me-please"

[http.services.ocs]
machine_auth_apikey = "change-me-please"
Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/local/frontend-global.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ files_namespace = "/"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/"
machine_auth_apikey = "change-me-please"

[http.services.ocs]

Expand Down
1 change: 0 additions & 1 deletion tests/oc-integration-tests/local/frontend.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ files_namespace = "/users/{{.Id.OpaqueId}}"
# - the oc js sdk is hardcoded to the remote.php/webdav so it will see the new tree
# - TODO android? no sync ... but will see different tree
webdav_namespace = "/users/{{.Id.OpaqueId}}"
machine_auth_apikey = "change-me-please"

# serve /ocs which contains the sharing and user provisioning api of owncloud classic
[http.services.ocs]
Expand Down

0 comments on commit b0ea559

Please sign in to comment.