diff --git a/changelog/unreleased/forbid-duplicate-shares.md b/changelog/unreleased/forbid-duplicate-shares.md new file mode 100644 index 0000000000..eae35d7017 --- /dev/null +++ b/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 diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index eea2e294f5..9fc36a1f42 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/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() diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go index 9d390d209e..7f166064d6 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go +++ b/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() {