From 5d33e2690e73877ed14a8be6bfefb7ad2816adff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 24 Jun 2022 10:51:01 +0200 Subject: [PATCH 1/9] Prevent recursive copy/move operations on the API level --- internal/http/services/owncloud/ocdav/copy.go | 26 ++++++++++++----- internal/http/services/owncloud/ocdav/move.go | 24 ++++++++++++---- .../http/services/owncloud/ocdav/ocdav.go | 28 +++++++++++++++++++ 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index d38d3f0d64..6c710f7847 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -486,6 +486,25 @@ 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 { + 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 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 { @@ -513,13 +532,6 @@ 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 { diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 562df4b254..e7ec160ad4 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -132,19 +132,31 @@ 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) { - oh := r.Header.Get(net.HeaderOverwrite) - log.Debug().Str("overwrite", oh).Msg("move") + client, err := s.getClient() + if err != nil { + log.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) + return + } - overwrite, err := net.ParseOverwrite(oh) + isChild, err := s.referenceIsChildOf(ctx, client, dst, src) if err != nil { + 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 } - client, err := s.getClient() + oh := r.Header.Get(net.HeaderOverwrite) + log.Debug().Str("overwrite", oh).Msg("move") + + overwrite, err := net.ParseOverwrite(oh) if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) + w.WriteHeader(http.StatusBadRequest) return } diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 1de9831c88..acf1b63cf3 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -27,8 +27,10 @@ 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" @@ -42,6 +44,7 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/favorite/registry" "github.com/cs3org/reva/v2/pkg/storage/utils/templates" 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" @@ -384,3 +387,28 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) { headers.Set("Strict-Transport-Security", "max-age=63072000") } } + +func (s *svc) referenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) { + if utils.ResourceIDEqual(child.ResourceId, parent.ResourceId) { + return strings.HasPrefix(child.Path, parent.Path+"/"), nil // Relative to the same resource -> compare paths + } + + if child.ResourceId.StorageId != parent.ResourceId.StorageId { + return false, nil // Not on the same storage -> not a child + } + + // 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 { + return false, err + } + parentPathRes, err := client.GetPath(ctx, &provider.GetPathRequest{ResourceId: parent.ResourceId}) + if err != nil { + return false, err + } + + return strings.HasPrefix( + path.Join(childPathRes.Path, child.Path), + path.Join(parentPathRes.Path, parent.Path)+"/"), nil +} From ca09131ff1b0806b4eddc8a9426a6a9c692f276d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 24 Jun 2022 11:07:27 +0200 Subject: [PATCH 2/9] Add changelog --- changelog/unreleased/prevent-recursive-copy-move.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/prevent-recursive-copy-move.md diff --git a/changelog/unreleased/prevent-recursive-copy-move.md b/changelog/unreleased/prevent-recursive-copy-move.md new file mode 100644 index 0000000000..9ce85b6119 --- /dev/null +++ b/changelog/unreleased/prevent-recursive-copy-move.md @@ -0,0 +1,5 @@ +Enhancement: Prevent recursive copy/move operations + +We changed the ocs API to prevent copying or moving a folder into one of its children. + +https://github.com/cs3org/reva/pull/3009 From 47595a6b34b9dd2bf468b397e47fe6f7e2729e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 1 Jul 2022 09:12:37 +0200 Subject: [PATCH 3/9] Enable machine auth in ocdav --- internal/http/services/owncloud/ocdav/ocdav.go | 2 ++ pkg/micro/ocdav/option.go | 7 +++++++ tests/oc-integration-tests/drone/frontend-global.toml | 1 + tests/oc-integration-tests/drone/frontend.toml | 1 + tests/oc-integration-tests/local-mesh/frontend-global.toml | 1 + tests/oc-integration-tests/local-mesh/frontend.toml | 1 + tests/oc-integration-tests/local/combined.toml | 1 + tests/oc-integration-tests/local/frontend-global.toml | 1 + tests/oc-integration-tests/local/frontend.toml | 1 + 9 files changed, 16 insertions(+) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index acf1b63cf3..b97f25138e 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -110,6 +110,8 @@ 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() { diff --git a/pkg/micro/ocdav/option.go b/pkg/micro/ocdav/option.go index 386542e639..baf834088a 100644 --- a/pkg/micro/ocdav/option.go +++ b/pkg/micro/ocdav/option.go @@ -84,6 +84,13 @@ 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) { diff --git a/tests/oc-integration-tests/drone/frontend-global.toml b/tests/oc-integration-tests/drone/frontend-global.toml index 40ba34506d..1d29cdf233 100644 --- a/tests/oc-integration-tests/drone/frontend-global.toml +++ b/tests/oc-integration-tests/drone/frontend-global.toml @@ -45,6 +45,7 @@ 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] diff --git a/tests/oc-integration-tests/drone/frontend.toml b/tests/oc-integration-tests/drone/frontend.toml index 35e5d2f92a..3f4eac85dd 100644 --- a/tests/oc-integration-tests/drone/frontend.toml +++ b/tests/oc-integration-tests/drone/frontend.toml @@ -46,6 +46,7 @@ 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" diff --git a/tests/oc-integration-tests/local-mesh/frontend-global.toml b/tests/oc-integration-tests/local-mesh/frontend-global.toml index 255acfeea6..c9ef145466 100644 --- a/tests/oc-integration-tests/local-mesh/frontend-global.toml +++ b/tests/oc-integration-tests/local-mesh/frontend-global.toml @@ -45,6 +45,7 @@ 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] diff --git a/tests/oc-integration-tests/local-mesh/frontend.toml b/tests/oc-integration-tests/local-mesh/frontend.toml index 2e6f5fdd9a..0e23a75375 100644 --- a/tests/oc-integration-tests/local-mesh/frontend.toml +++ b/tests/oc-integration-tests/local-mesh/frontend.toml @@ -46,6 +46,7 @@ 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] diff --git a/tests/oc-integration-tests/local/combined.toml b/tests/oc-integration-tests/local/combined.toml index 1eab1b89e1..bcc158b1a1 100644 --- a/tests/oc-integration-tests/local/combined.toml +++ b/tests/oc-integration-tests/local/combined.toml @@ -119,6 +119,7 @@ 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" diff --git a/tests/oc-integration-tests/local/frontend-global.toml b/tests/oc-integration-tests/local/frontend-global.toml index 7c62831589..548b6f1d4c 100644 --- a/tests/oc-integration-tests/local/frontend-global.toml +++ b/tests/oc-integration-tests/local/frontend-global.toml @@ -49,6 +49,7 @@ 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] diff --git a/tests/oc-integration-tests/local/frontend.toml b/tests/oc-integration-tests/local/frontend.toml index f4b0f1ec32..8bdfa766ca 100644 --- a/tests/oc-integration-tests/local/frontend.toml +++ b/tests/oc-integration-tests/local/frontend.toml @@ -50,6 +50,7 @@ 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] From fee4ad145e6d709b71bee4745a86f8aafd7650a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 1 Jul 2022 10:00:04 +0200 Subject: [PATCH 4/9] Fix error message --- internal/http/services/owncloud/ocdav/copy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 6c710f7847..d66ad6bd17 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -495,7 +495,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re isChild, err := s.referenceIsChildOf(ctx, client, dstRef, srcRef) if err != nil { - log.Error().Err(err).Msg("error while trying to detect recursive move operation") + log.Error().Err(err).Msg("error while trying to detect recursive copy operation") w.WriteHeader(http.StatusInternalServerError) } if isChild { From 1fb00ce5cf2cbd424d625d2a2e8eaa79c8632b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 1 Jul 2022 10:01:19 +0200 Subject: [PATCH 5/9] Also detect recursive operations when the SSP is involved --- .../http/services/owncloud/ocdav/ocdav.go | 86 +++++++++++++++++-- 1 file changed, 81 insertions(+), 5 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index b97f25138e..2e485c5774 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -43,11 +43,15 @@ 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. @@ -390,19 +394,91 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) { } } +func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) { + // 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 child.ResourceId.StorageId != parent.ResourceId.StorageId { - return false, nil // Not on the same storage -> not a child + 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}) @@ -410,7 +486,7 @@ func (s *svc) referenceIsChildOf(ctx context.Context, client gatewayv1beta1.Gate return false, err } - return strings.HasPrefix( - path.Join(childPathRes.Path, child.Path), - path.Join(parentPathRes.Path, parent.Path)+"/"), nil + cp := path.Join(childPathRes.Path, child.Path) + "/" + pp := path.Join(parentPathRes.Path, parent.Path) + "/" + return strings.HasPrefix(cp, pp), nil } From d6ac3924f2a8110453db484cf25caf8057ac5544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 1 Jul 2022 10:24:37 +0200 Subject: [PATCH 6/9] Fix hound issues --- internal/http/services/owncloud/ocdav/ocdav.go | 6 +++--- pkg/micro/ocdav/option.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 2e485c5774..58531cc16e 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -115,7 +115,7 @@ type Config struct { ProductName string `mapstructure:"product_name"` ProductVersion string `mapstructure:"product_version"` - MachineAuthApiKey string `mapstructure:"machine_auth_apikey"` + MachineAuthAPIKey string `mapstructure:"machine_auth_apikey"` } func (c *Config) init() { @@ -418,7 +418,7 @@ func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.G if err != nil { return false, err } - parentAuthCtx, err := authContextForUser(client, parentStatRes.Info.Owner, s.c.MachineAuthApiKey) + parentAuthCtx, err := authContextForUser(client, parentStatRes.Info.Owner, s.c.MachineAuthAPIKey) if err != nil { return false, err } @@ -441,7 +441,7 @@ func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.G return false, err } } - childAuthCtx, err := authContextForUser(client, childStatRes.Info.Owner, s.c.MachineAuthApiKey) + childAuthCtx, err := authContextForUser(client, childStatRes.Info.Owner, s.c.MachineAuthAPIKey) if err != nil { return false, err } diff --git a/pkg/micro/ocdav/option.go b/pkg/micro/ocdav/option.go index baf834088a..2c94830440 100644 --- a/pkg/micro/ocdav/option.go +++ b/pkg/micro/ocdav/option.go @@ -84,10 +84,10 @@ func JWTSecret(s string) Option { } } -// MachineAuthApiKey provides a function to set the machine auth api key option. -func MachineAuthApiKey(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 + o.config.MachineAuthAPIKey = s } } From 6d4078879e7f29a4e827f03ec26192277402910d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 4 Jul 2022 11:19:53 +0200 Subject: [PATCH 7/9] Set the owner on the stat response to the share jail --- .../sharesstorageprovider/sharesstorageprovider.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 196cb1108e..83bb178e83 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -38,6 +38,7 @@ 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" @@ -662,6 +663,10 @@ 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 @@ -690,7 +695,8 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide PermissionSet: &provider.ResourcePermissions{ // TODO }, - Etag: shareMd[earliestShare.Id.OpaqueId].ETag, + Etag: shareMd[earliestShare.Id.OpaqueId].ETag, + Owner: owner.Id, }, }, nil } From f44e9e81214c6bb0d5c4599cb27057256b9b932c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 4 Jul 2022 11:22:20 +0200 Subject: [PATCH 8/9] Implement GetPath() for the share jail root --- .../sharesstorageprovider/sharesstorageprovider.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 83bb178e83..ef17ad92ce 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -273,6 +273,14 @@ 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") } From 102e7cf7e028d232ade19c20858f1348226960c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 4 Jul 2022 16:20:59 +0200 Subject: [PATCH 9/9] Return a better error when the ocdav machine auth config is missing --- internal/http/services/owncloud/ocdav/copy.go | 11 +++++++++-- internal/http/services/owncloud/ocdav/move.go | 11 +++++++++-- internal/http/services/owncloud/ocdav/ocdav.go | 3 +++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index d66ad6bd17..335617b5bc 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -35,6 +35,7 @@ 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" @@ -495,8 +496,14 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re isChild, err := s.referenceIsChildOf(ctx, client, dstRef, srcRef) if err != nil { - log.Error().Err(err).Msg("error while trying to detect recursive copy operation") - w.WriteHeader(http.StatusInternalServerError) + 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) diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index e7ec160ad4..85a7ed9b5b 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -30,6 +30,7 @@ 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" @@ -141,8 +142,14 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req isChild, err := s.referenceIsChildOf(ctx, client, dst, src) if err != nil { - log.Error().Err(err).Msg("error while trying to detect recursive move operation") - w.WriteHeader(http.StatusInternalServerError) + 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) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 58531cc16e..f107647ee8 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -395,6 +395,9 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) { } 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})