From 40db8d28d8a92b5a50d42e4ce650d116864f804c Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 20 Oct 2020 14:54:44 +0200 Subject: [PATCH 1/3] update grants in the storage provider on share update Signed-off-by: David Christofas --- .../update-grants-on-share-update.md | 5 + .../services/gateway/usershareprovider.go | 93 +++++++++++-------- 2 files changed, 61 insertions(+), 37 deletions(-) create mode 100644 changelog/unreleased/update-grants-on-share-update.md diff --git a/changelog/unreleased/update-grants-on-share-update.md b/changelog/unreleased/update-grants-on-share-update.md new file mode 100644 index 0000000000..72ad730341 --- /dev/null +++ b/changelog/unreleased/update-grants-on-share-update.md @@ -0,0 +1,5 @@ +Bugfix: update share grants on share update + +When a share was updated the share information in the share manager was updated but the grants set by the storage provider were not. + +https://github.com/cs3org/reva/pull/1258 diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 29ca23f1c0..9e87731b97 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -186,46 +186,31 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq } // TODO(labkode): if both commits are enabled they could be done concurrently. - /* - if s.c.CommitShareToStorageGrant { - getShareReq := &collaboration.GetShareRequest{ - Ref: req.Ref, - } - getShareRes, err := c.GetShare(ctx, getShareReq) - if err != nil { - return nil, errors.Wrap(err, "gateway: error calling GetShare") - } - if getShareRes.Status.Code != rpc.Code_CODE_OK { - return &collaboration.UpdateShareResponse{ - Status: status.NewInternal(ctx, status.NewErrorFromCode(getShareRes.Status.Code, "gateway"), - "error getting share when committing to the share"), - }, nil - } + if s.c.CommitShareToStorageGrant { + getShareReq := &collaboration.GetShareRequest{ + Ref: req.Ref, + } + getShareRes, err := c.GetShare(ctx, getShareReq) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling GetShare") + } - grantReq := &provider.UpdateGrantRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Id{ - Id: getShareRes.Share.ResourceId, - }, - }, - Grant: &provider.Grant{ - Grantee: getShareRes.Share.Grantee, - Permissions: getShareRes.Share.Permissions.Permissions, - }, - } - grantRes, err := s.UpdateGrant(ctx, grantReq) - if err != nil { - return nil, errors.Wrap(err, "gateway: error calling UpdateGrant") - } - if grantRes.Status.Code != rpc.Code_CODE_OK { - return &collaboration.UpdateShareResponse{ - Status: status.NewInternal(ctx, status.NewErrorFromCode(grantRes.Status.Code, "gateway"), - "error updating storage grant"), - }, nil - } + if getShareRes.Status.Code != rpc.Code_CODE_OK { + return &collaboration.UpdateShareResponse{ + Status: status.NewInternal(ctx, status.NewErrorFromCode(getShareRes.Status.Code, "gateway"), + "error getting share when committing to the share"), + }, nil + } + updateGrantStatus, err := s.updateGrant(ctx, getShareRes.GetShare().GetResourceId(), + getShareRes.GetShare().GetGrantee(), + getShareRes.GetShare().GetPermissions().GetPermissions()) + if updateGrantStatus.Code != rpc.Code_CODE_OK { + return &collaboration.UpdateShareResponse{ + Status: updateGrantStatus, + }, err } - */ + } return res, nil } @@ -464,6 +449,40 @@ func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider return status.NewOK(ctx), nil } +func (s *svc) updateGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions) (*rpc.Status, error) { + + grantReq := &provider.UpdateGrantRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Id{ + Id: id, + }, + }, + Grant: &provider.Grant{ + Grantee: g, + Permissions: p, + }, + } + + c, err := s.findByID(ctx, id) + if err != nil { + if _, ok := err.(errtypes.IsNotFound); ok { + return status.NewNotFound(ctx, "storage provider not found"), nil + } + return status.NewInternal(ctx, err, "error finding storage provider"), nil + } + + grantRes, err := c.UpdateGrant(ctx, grantReq) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling UpdateGrant") + } + if grantRes.Status.Code != rpc.Code_CODE_OK { + return status.NewInternal(ctx, status.NewErrorFromCode(grantRes.Status.Code, "gateway"), + "error committing share to storage grant"), nil + } + + return status.NewOK(ctx), nil +} + func (s *svc) removeGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions) (*rpc.Status, error) { grantReq := &provider.RemoveGrantRequest{ From a789c6d7c4945a4b84c6da6b9eedcccbbbf6821a Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 20 Oct 2020 15:12:49 +0200 Subject: [PATCH 2/3] add error check Signed-off-by: David Christofas --- internal/grpc/services/gateway/usershareprovider.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 9e87731b97..35ef3960b2 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -205,10 +205,15 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq updateGrantStatus, err := s.updateGrant(ctx, getShareRes.GetShare().GetResourceId(), getShareRes.GetShare().GetGrantee(), getShareRes.GetShare().GetPermissions().GetPermissions()) + + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling updateGrant") + } + if updateGrantStatus.Code != rpc.Code_CODE_OK { return &collaboration.UpdateShareResponse{ Status: updateGrantStatus, - }, err + }, nil } } From 2b2a6f67e05248cb23c098a111a0fc8b9afe44ab Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 20 Oct 2020 15:47:39 +0200 Subject: [PATCH 3/3] update expected failures using OWNCLOUD storage Signed-off-by: David Christofas --- .../expected-failures-on-OWNCLOUD-storage.txt | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt index 0eaadb2d9a..a9a816c3a2 100644 --- a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt +++ b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt @@ -609,20 +609,9 @@ apiShareReshareToShares2/reShareSubfolder.feature:84 # # https://github.com/owncloud/product/issues/270 share permissions are not enforced # -apiShareReshareToShares3/reShareUpdate.feature:25 -apiShareReshareToShares3/reShareUpdate.feature:26 apiShareReshareToShares3/reShareUpdate.feature:59 apiShareReshareToShares3/reShareUpdate.feature:60 # -# WebDav::listFolderAndReturnResponseXml Received empty response where XML was expected (Exception) (note: passes in owncloud/ocis) -# -apiShareReshareToShares3/reShareUpdate.feature:42 -apiShareReshareToShares3/reShareUpdate.feature:43 -apiShareReshareToShares3/reShareUpdate.feature:76 -apiShareReshareToShares3/reShareUpdate.feature:77 -apiShareReshareToShares3/reShareUpdate.feature:93 -apiShareReshareToShares3/reShareUpdate.feature:94 -# # https://github.com/owncloud/product/issues/207 Response is empty when accepting a share # apiShareReshareToShares1/reShare.feature:54