update grants in the storage provider on share update (#1258)

cs3org · Oct 20, 2020 · 05ab93e · 05ab93e
1 parent 86b2550
commit 05ab93e
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 48 deletions.
diff --git a/changelog/unreleased/update-grants-on-share-update.md 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
@@ -186,46 +186,36 @@ 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 err != nil {
+			return nil, errors.Wrap(err, "gateway: error calling updateGrant")
 		}
-	*/
+
+		if updateGrantStatus.Code != rpc.Code_CODE_OK {
+			return &collaboration.UpdateShareResponse{
+				Status: updateGrantStatus,
+			}, nil
+		}
+	}
 
 	return res, nil
 }
@@ -464,6 +454,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{

diff --git 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