From 50578f401cffd8d7e9492c074a2ff2eadeae9b8f Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 4 Jun 2024 16:59:01 +0200 Subject: [PATCH] [full-ci][bump reva] fixed the response code when the role/permission is empty on the share update --- changelog/unreleased/fix-share-update.md | 6 +++ go.mod | 2 +- go.sum | 4 +- ...ected-failures-localAPI-on-OCIS-storage.md | 3 -- .../handlers/apps/sharing/shares/shares.go | 3 ++ .../v2/pkg/storage/fs/posix/lookup/lookup.go | 48 +++++++++---------- .../storage/fs/posix/lookup/store_idcache.go | 2 +- .../pkg/storage/fs/posix/tree/assimilation.go | 12 +++-- .../storage/utils/decomposedfs/node/xattrs.go | 6 +++ .../pkg/storage/utils/decomposedfs/spaces.go | 3 +- .../pkg/storage/utils/decomposedfs/upload.go | 9 ++-- vendor/modules.txt | 2 +- 12 files changed, 55 insertions(+), 45 deletions(-) create mode 100644 changelog/unreleased/fix-share-update.md diff --git a/changelog/unreleased/fix-share-update.md b/changelog/unreleased/fix-share-update.md new file mode 100644 index 0000000000..b4bbfaf11c --- /dev/null +++ b/changelog/unreleased/fix-share-update.md @@ -0,0 +1,6 @@ +Bugfix: Fix share update + +We fixed the response code when the role/permission is empty on the share update + +https://github.com/owncloud/ocis/pull/9301 +https://github.com/owncloud/ocis/issues/8747 diff --git a/go.mod b/go.mod index bcd7e8764e..d6f6d4033e 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/cenkalti/backoff v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.10.0 github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 - github.com/cs3org/reva/v2 v2.19.2-0.20240603112905-634bf103c8be + github.com/cs3org/reva/v2 v2.19.2-0.20240604132648-408bb6433068 github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e github.com/egirna/icap-client v0.1.1 diff --git a/go.sum b/go.sum index 8578b3b669..1e69bb7ee4 100644 --- a/go.sum +++ b/go.sum @@ -1025,8 +1025,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c= github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= -github.com/cs3org/reva/v2 v2.19.2-0.20240603112905-634bf103c8be h1:iD1L1MEeoLieeAcoa9iWKVdXHhhftCtJVjOmBRxn8y0= -github.com/cs3org/reva/v2 v2.19.2-0.20240603112905-634bf103c8be/go.mod h1:lKqw0VuP1NcZbhj0e6tGoAGq3tgWO/pLafVJyDK0yVI= +github.com/cs3org/reva/v2 v2.19.2-0.20240604132648-408bb6433068 h1:DAmvibMtV7HxsQoG3jfwm78XftA/js0ECuv1pelSON8= +github.com/cs3org/reva/v2 v2.19.2-0.20240604132648-408bb6433068/go.mod h1:lKqw0VuP1NcZbhj0e6tGoAGq3tgWO/pLafVJyDK0yVI= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index b85584b7ef..929c40deb4 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -299,9 +299,6 @@ The expected failures in this file are from features in the owncloud/ocis repo. - [apiSpacesDavOperation/moveByFileId.feature:208](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L208) - [apiSpacesDavOperation/moveByFileId.feature:209](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L209) -### [500 when deleting share role](https://github.com/owncloud/ocis/issues/8747) - -- [apiSpacesShares/shareSubItemOfSpace.feature:159](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesShares/shareSubItemOfSpace.feature#L159) - Note: always have an empty line at the end of this file. The bash script that processes this file requires that the last line has a newline on the end. diff --git a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index ef88c9acb8..b006d5d528 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -859,6 +859,9 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, share *col case rpc.Code_CODE_LOCKED: response.WriteOCSError(w, r, response.MetaLocked.StatusCode, uRes.GetStatus().GetMessage(), nil) return + case rpc.Code_CODE_INVALID_ARGUMENT, rpc.Code_CODE_FAILED_PRECONDITION: + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, uRes.GetStatus().GetMessage(), nil) + return } response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update share request failed", err) return diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/lookup.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/lookup.go index 7dc9780db5..9fe369c30f 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/lookup.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/lookup.go @@ -98,7 +98,21 @@ func (lu *Lookup) GetCachedID(ctx context.Context, spaceID, nodeID string) (stri func (lu *Lookup) WarmupIDCache(root string) error { spaceID := []byte("") - var gid int + scopeSpace := func(spaceCandidate string) error { + if !lu.Options.UseSpaceGroups { + return nil + } + + // set the uid and gid for the space + fi, err := os.Stat(spaceCandidate) + if err != nil { + return err + } + sys := fi.Sys().(*syscall.Stat_t) + gid := int(sys.Gid) + _, err = lu.userMapper.ScopeUserByIds(-1, gid) + return err + } return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -107,41 +121,23 @@ func (lu *Lookup) WarmupIDCache(root string) error { attribs, err := lu.metadataBackend.All(context.Background(), path) if err == nil { - nodeSpaceID, ok := attribs[prefixes.SpaceIDAttr] - if ok { + nodeSpaceID := attribs[prefixes.SpaceIDAttr] + if len(nodeSpaceID) > 0 { spaceID = nodeSpaceID - // set the uid and gid for the space - fi, err := os.Stat(path) + err = scopeSpace(path) if err != nil { return err } - sys := fi.Sys().(*syscall.Stat_t) - gid = int(sys.Gid) - _, err = lu.userMapper.ScopeUserByIds(-1, gid) - if err != nil { - return err - } - } - - if len(spaceID) == 0 { + } else { // try to find space spaceCandidate := path for strings.HasPrefix(spaceCandidate, lu.Options.Root) { spaceID, err = lu.MetadataBackend().Get(context.Background(), spaceCandidate, prefixes.SpaceIDAttr) if err == nil { - if lu.Options.UseSpaceGroups { - // set the uid and gid for the space - fi, err := os.Stat(spaceCandidate) - if err != nil { - return err - } - sys := fi.Sys().(*syscall.Stat_t) - gid := int(sys.Gid) - _, err = lu.userMapper.ScopeUserByIds(-1, gid) - if err != nil { - return err - } + err = scopeSpace(path) + if err != nil { + return err } break } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/store_idcache.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/store_idcache.go index a7690052c0..b695cde61a 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/store_idcache.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/lookup/store_idcache.go @@ -58,7 +58,7 @@ func (c *StoreIDCache) Set(_ context.Context, spaceID, nodeID, val string) error // Get returns the value for a given key func (c *StoreIDCache) Get(_ context.Context, spaceID, nodeID string) (string, bool) { records, err := c.cache.Read(cacheKey(spaceID, nodeID)) - if err != nil { + if err != nil || len(records) == 0 { return "", false } return string(records[0].Value), true diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/tree/assimilation.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/tree/assimilation.go index 9070432ded..16f9de16f5 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/tree/assimilation.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/fs/posix/tree/assimilation.go @@ -217,15 +217,19 @@ assimilate: return nil, errors.Wrap(err, "failed to stat item") } - previousAttribs, err := t.lookup.MetadataBackend().All(context.Background(), path) + attrs, err := t.lookup.MetadataBackend().All(context.Background(), path) if err != nil && !metadata.IsAttrUnset(err) { return nil, errors.Wrap(err, "failed to get item attribs") } + previousAttribs := node.Attributes(attrs) attributes := node.Attributes{ - prefixes.IDAttr: []byte(id), - prefixes.NameAttr: []byte(filepath.Base(path)), - prefixes.MTimeAttr: []byte(fi.ModTime().Format(time.RFC3339)), + prefixes.IDAttr: []byte(id), + prefixes.NameAttr: []byte(filepath.Base(path)), + } + prevMtime, err := previousAttribs.Time(prefixes.MTimeAttr) + if err != nil || prevMtime.Before(fi.ModTime()) { + attributes[prefixes.MTimeAttr] = []byte(fi.ModTime().Format(time.RFC3339Nano)) } if len(parentID) > 0 { attributes[prefixes.ParentidAttr] = []byte(parentID) diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/xattrs.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/xattrs.go index 79ba71f708..0e5d186b34 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/xattrs.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/xattrs.go @@ -22,6 +22,7 @@ import ( "context" "io" "strconv" + "time" "github.com/pkg/xattr" ) @@ -59,6 +60,11 @@ func (md Attributes) SetUInt64(key string, val uint64) { md[key] = []byte(strconv.FormatUint(val, 10)) } +// Time reads a time value +func (md Attributes) Time(key string) (time.Time, error) { + return time.Parse(time.RFC3339Nano, string(md[key])) +} + // SetXattrs sets multiple extended attributes on the write-through cache/node func (n *Node) SetXattrsWithContext(ctx context.Context, attribs map[string][]byte, acquireLock bool) (err error) { if n.xattrsCache != nil { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go index c8e644ee12..a635db2da0 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go @@ -325,7 +325,8 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide if requestedUserID != nil { allMatches, err = fs.userSpaceIndex.Load(requestedUserID.GetOpaqueId()) - if err != nil { + // do not return an error if the user has no spaces + if err != nil && !os.IsNotExist(err) { return nil, errors.Wrap(err, "error reading user index") } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload.go index 95758fdbfb..33b077972c 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload.go @@ -24,7 +24,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "time" "github.com/google/uuid" @@ -181,12 +180,10 @@ func (fs *Decomposedfs) InitiateUpload(ctx context.Context, ref *provider.Refere session.SetStorageValue("SpaceRoot", n.SpaceRoot.ID) // TODO SpaceRoot -> SpaceID session.SetStorageValue("SpaceOwnerOrManager", n.SpaceOwnerOrManager(ctx).GetOpaqueId()) // TODO needed for what? - // remember the gid of the space - fi, err := os.Stat(n.SpaceRoot.InternalPath()) - if err != nil { - return nil, err + spaceGID, ok := ctx.Value(CtxKeySpaceGID).(uint32) + if ok { + session.SetStorageValue("SpaceGid", fmt.Sprintf("%d", spaceGID)) } - session.SetStorageValue("SpaceGid", fmt.Sprintf("%d", (fi.Sys().(*syscall.Stat_t).Gid))) iid, _ := ctxpkg.ContextGetInitiator(ctx) session.SetMetadata("initiatorid", iid) diff --git a/vendor/modules.txt b/vendor/modules.txt index 2d623d6b62..b685532750 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -366,7 +366,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.19.2-0.20240603112905-634bf103c8be +# github.com/cs3org/reva/v2 v2.19.2-0.20240604132648-408bb6433068 ## explicit; go 1.21 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime