From 283b9a9d476662882352e984d587ee52f891cb1f Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 3 Nov 2022 17:32:10 +0100 Subject: [PATCH 1/4] remove share jail check Signed-off-by: Christian Richter --- pkg/storage/utils/decomposedfs/lookup/lookup.go | 5 ----- pkg/storage/utils/decomposedfs/node/node.go | 1 - pkg/storage/utils/decomposedfs/node/permissions.go | 6 ------ pkg/storage/utils/decomposedfs/options/options.go | 9 --------- pkg/storage/utils/decomposedfs/tree/tree.go | 1 - 5 files changed, 22 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/lookup/lookup.go b/pkg/storage/utils/decomposedfs/lookup/lookup.go index da19dc14d6..c4beaa1791 100644 --- a/pkg/storage/utils/decomposedfs/lookup/lookup.go +++ b/pkg/storage/utils/decomposedfs/lookup/lookup.go @@ -172,8 +172,3 @@ func (lu *Lookup) InternalRoot() string { func (lu *Lookup) InternalPath(spaceID, nodeID string) string { return filepath.Join(lu.Options.Root, "spaces", Pathify(spaceID, 1, 2), "nodes", Pathify(nodeID, 4, 2)) } - -// ShareFolder returns the internal storage root directory -func (lu *Lookup) ShareFolder() string { - return lu.Options.ShareFolder -} diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index cfaffd8806..714c54a07e 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -90,7 +90,6 @@ type PathLookup interface { InternalRoot() string InternalPath(spaceID, nodeID string) string Path(ctx context.Context, n *Node) (path string, err error) - ShareFolder() string } // New returns a new instance of Node diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index 8ab3096161..117b9a72f3 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -88,17 +88,11 @@ func NewPermissions(lu PathLookup) *Permissions { func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap provider.ResourcePermissions, err error) { u, ok := ctxpkg.ContextGetUser(ctx) if !ok { - appctx.GetLogger(ctx).Debug().Interface("node", n.ID).Msg("no user in context, returning default permissions") return NoPermissions(), nil } // check if the current user is the owner if utils.UserIDEqual(u.Id, n.Owner()) { - lp, err := n.lu.Path(ctx, n) - if err == nil && lp == n.lu.ShareFolder() { - return ShareFolderPermissions(), nil - } - appctx.GetLogger(ctx).Debug().Str("node", n.ID).Msg("user is owner, returning owner permissions") return OwnerPermissions(), nil } // determine root diff --git a/pkg/storage/utils/decomposedfs/options/options.go b/pkg/storage/utils/decomposedfs/options/options.go index 0370456f30..705f9c8638 100644 --- a/pkg/storage/utils/decomposedfs/options/options.go +++ b/pkg/storage/utils/decomposedfs/options/options.go @@ -39,9 +39,6 @@ type Options struct { // UserLayout describes the relative path from the storage's root node to the users home node. UserLayout string `mapstructure:"user_layout"` - // TODO NodeLayout option to save nodes as eg. nodes/1d/d8/1dd84abf-9466-4e14-bb86-02fc4ea3abcf - ShareFolder string `mapstructure:"share_folder"` - // propagate mtime changes as tmtime (tree modification time) to the parent directory when user.ocis.propagation=1 is set on a node TreeTimeAccounting bool `mapstructure:"treetime_accounting"` @@ -73,12 +70,6 @@ func New(m map[string]interface{}) (*Options, error) { // ensure user layout has no starting or trailing / o.UserLayout = strings.Trim(o.UserLayout, "/") - if o.ShareFolder == "" { - o.ShareFolder = "/Shares" - } - // ensure share folder always starts with slash - o.ShareFolder = filepath.Join("/", o.ShareFolder) - // c.DataDirectory should never end in / unless it is the root o.Root = filepath.Clean(o.Root) diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index b6337d931a..8c6724d6fb 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -60,7 +60,6 @@ type PathLookup interface { InternalRoot() string InternalPath(spaceID, nodeID string) string Path(ctx context.Context, n *node.Node) (path string, err error) - ShareFolder() string } // Tree manages a hierarchical tree From 71e3c2fafebc99fd48a2a618f9e934614d736a05 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 3 Nov 2022 17:34:26 +0100 Subject: [PATCH 2/4] add changelog Signed-off-by: Christian Richter --- changelog/unreleased/remove-share-jail-check.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/remove-share-jail-check.md diff --git a/changelog/unreleased/remove-share-jail-check.md b/changelog/unreleased/remove-share-jail-check.md new file mode 100644 index 0000000000..2690a9c417 --- /dev/null +++ b/changelog/unreleased/remove-share-jail-check.md @@ -0,0 +1,6 @@ +Bug: Remove share jail fix + +We have removed the share jail check. + +https://github.com/cs3org/reva/pull/3432 +https://github.com/owncloud/ocis/issues/4945 \ No newline at end of file From 28b7f433b8b35607e4004060a2bcb6e858451b57 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 3 Nov 2022 17:45:54 +0100 Subject: [PATCH 3/4] remove reference creation code Signed-off-by: Christian Richter --- .../unreleased/remove-share-jail-check.md | 2 +- .../utils/decomposedfs/decomposedfs.go | 82 +------------------ 2 files changed, 2 insertions(+), 82 deletions(-) diff --git a/changelog/unreleased/remove-share-jail-check.md b/changelog/unreleased/remove-share-jail-check.md index 2690a9c417..a568f5b425 100644 --- a/changelog/unreleased/remove-share-jail-check.md +++ b/changelog/unreleased/remove-share-jail-check.md @@ -1,4 +1,4 @@ -Bug: Remove share jail fix +Bugfix: Remove share jail fix We have removed the share jail check. diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 352d2ae51b..d964837c8c 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -30,7 +30,6 @@ import ( "path" "path/filepath" "strconv" - "strings" "syscall" cs3permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" @@ -53,7 +52,6 @@ import ( "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/pkg/errors" - "go.opentelemetry.io/otel/codes" "google.golang.org/grpc" ) @@ -403,85 +401,7 @@ func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference) // To mimic the eos and owncloud driver we only allow references as children of the "/Shares" folder // FIXME: This comment should explain briefly what a reference is in this context. func (fs *Decomposedfs) CreateReference(ctx context.Context, p string, targetURI *url.URL) (err error) { - ctx, span := appctx.GetTracerProvider(ctx).Tracer("reva").Start(ctx, "CreateReference") - defer span.End() - - p = strings.Trim(p, "/") - parts := strings.Split(p, "/") - - if len(parts) != 2 { - err := errtypes.PermissionDenied("Decomposedfs: references must be a child of the share folder: share_folder=" + fs.o.ShareFolder + " path=" + p) - span.SetStatus(codes.Error, err.Error()) - return err - } - - if parts[0] != strings.Trim(fs.o.ShareFolder, "/") { - err := errtypes.PermissionDenied("Decomposedfs: cannot create references outside the share folder: share_folder=" + fs.o.ShareFolder + " path=" + p) - span.SetStatus(codes.Error, err.Error()) - return err - } - - // create Shares folder if it does not exist - var parentNode *node.Node - var parentCreated, childCreated bool // defaults to false - if parentNode, err = fs.lu.NodeFromResource(ctx, &provider.Reference{Path: fs.o.ShareFolder}); err != nil { - err := errtypes.InternalError(err.Error()) - span.SetStatus(codes.Error, err.Error()) - return err - } else if !parentNode.Exists { - if err = fs.tp.CreateDir(ctx, parentNode); err != nil { - span.SetStatus(codes.Error, err.Error()) - return err - } - parentCreated = true - } - - var childNode *node.Node - // clean up directories created here on error - defer func() { - if err != nil { - // do not catch the error to not shadow the original error - if childCreated && childNode != nil { - if tmpErr := fs.tp.Delete(ctx, childNode); tmpErr != nil { - appctx.GetLogger(ctx).Error().Err(tmpErr).Str("node_id", childNode.ID).Msg("Can not clean up child node after error") - } - } - if parentCreated && parentNode != nil { - if tmpErr := fs.tp.Delete(ctx, parentNode); tmpErr != nil { - appctx.GetLogger(ctx).Error().Err(tmpErr).Str("node_id", parentNode.ID).Msg("Can not clean up parent node after error") - } - - } - } - }() - - if childNode, err = parentNode.Child(ctx, parts[1]); err != nil { - return errtypes.InternalError(err.Error()) - } - - if childNode.Exists { - // TODO append increasing number to mountpoint name - err := errtypes.AlreadyExists(p) - span.SetStatus(codes.Error, err.Error()) - return err - } - - if err := fs.tp.CreateDir(ctx, childNode); err != nil { - span.SetStatus(codes.Error, err.Error()) - return err - } - childCreated = true - - if err := childNode.SetMetadata(xattrs.ReferenceAttr, targetURI.String()); err != nil { - // the reference could not be set - that would result in an lost reference? - err := errors.Wrapf(err, "Decomposedfs: error setting the target %s on the reference file %s", - targetURI.String(), - childNode.InternalPath(), - ) - span.SetStatus(codes.Error, err.Error()) - return err - } - return nil + return errtypes.NotSupported("not implemented") } // Move moves a resource from one reference to another From 862c94a29f2154d743100373369cb89becc57fe3 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Fri, 4 Nov 2022 12:19:02 +0100 Subject: [PATCH 4/4] remove obsolete tests Signed-off-by: Christian Richter --- pkg/storage/utils/decomposedfs/options/options_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/storage/utils/decomposedfs/options/options_test.go b/pkg/storage/utils/decomposedfs/options/options_test.go index 5fbeca6c3f..dca13b570b 100644 --- a/pkg/storage/utils/decomposedfs/options/options_test.go +++ b/pkg/storage/utils/decomposedfs/options/options_test.go @@ -43,7 +43,6 @@ var _ = Describe("Options", func() { }) It("sets defaults", func() { - Expect(len(o.ShareFolder) > 0).To(BeTrue()) Expect(len(o.UserLayout) > 0).To(BeTrue()) })