Forbid duplicate shares

changelog/unreleased/forbid-duplicate-shares.md

@@ -0,0 +1,7 @@
+Bugfix: Forbid duplicate shares
+
+When sending a CreateShare request twice two shares would be created, one being not accessible.
+This was blocked by web so the issue wasn't obvious. Now it's forbidden to create share for a
+user who already has a share on that same resource
+
+https://github.com/cs3org/reva/pull/3176

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -1342,6 +1342,25 @@ func (h *Handler) getResourceInfo(ctx context.Context, client gateway.GatewayAPI
 }
 
 func (h *Handler) createCs3Share(ctx context.Context, w http.ResponseWriter, r *http.Request, client gateway.GatewayAPIClient, req *collaboration.CreateShareRequest) (*collaboration.Share, *ocsError) {
+	exists, err := h.granteeExists(ctx, req.Grant.Grantee, req.ResourceInfo.Id)
+	if err != nil {
+		return nil, &ocsError{
+			Code:    response.MetaServerError.StatusCode,
+			Message: "error sending a grpc list shares request",
+			Error:   err,
+		}
+	}
+
+	if exists {
+		// the grantee already has a share - should we jump to UpdateShare?
+		// for now - lets error
+		return nil, &ocsError{
+			Code:    response.MetaBadRequest.StatusCode,
+			Message: "grantee already has a share on this item",
+			Error:   nil,
+		}
+	}
+
 	createShareResponse, err := client.CreateShare(ctx, req)
 	if err != nil {
 		return nil, &ocsError{
@@ -1407,6 +1426,39 @@ func (h *Handler) getHomeNamespace(u *userpb.User) string {
 	return templates.WithUser(u, h.homeNamespace)
 }
 
+func (h *Handler) granteeExists(ctx context.Context, g *provider.Grantee, rid *provider.ResourceId) (bool, error) {
+	client, err := h.getClient()
+	if err != nil {
+		return false, err
+	}
+
+	lsreq := collaboration.ListSharesRequest{
+		Filters: []*collaboration.Filter{
+			{
+				Type: collaboration.Filter_TYPE_RESOURCE_ID,
+				Term: &collaboration.Filter_ResourceId{
+					ResourceId: rid,
+				},
+			},
+		},
+	}
+	lsres, err := client.ListShares(ctx, &lsreq)
+	if err != nil {
+		return false, err
+	}
+	if lsres.GetStatus().GetCode() != rpc.Code_CODE_OK {
+		return false, fmt.Errorf("unexpected status code from ListShares: %v", lsres.GetStatus())
+	}
+
+	for _, s := range lsres.GetShares() {
+		if utils.GranteeEqual(g, s.GetGrantee()) {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
 // sufficientPermissions returns true if the `existing` permissions contain the `requested` permissions
 func sufficientPermissions(existing, requested *provider.ResourcePermissions) bool {
 	ep := conversions.RoleFromResourcePermissions(existing).OCSPermissions()

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go

@@ -145,6 +145,10 @@ var _ = Describe("The ocs API", func() {
 				Status: status.NewOK(context.Background()),
 				Share:  share,
 			}, nil)
+
+			client.On("ListShares", mock.Anything, mock.Anything).Return(&collaboration.ListSharesResponse{
+				Status: status.NewOK(context.Background()),
+			}, nil)
 		})
 
 		Context("when there are no existing shares to the resource yet", func() {