usershareprovider: check permissions before updating a share

.drone.star

@@ -1,5 +1,5 @@
 # images
-OC_CI_GOLANG = "owncloudci/golang:1.20"
+OC_CI_GOLANG = "owncloudci/golang:1.21"
 OC_CI_ALPINE = "owncloudci/alpine:latest"
 OSIXIA_OPEN_LDAP = "osixia/openldap:1.3.0"
 REDIS = "redis:6-alpine"

.github/workflows/lint.yml

@@ -14,8 +14,9 @@ jobs:
       - name: Checkout
         uses: actions/[email protected]
       - name: Setup Go environment
-        uses: actions/setup-go@v3.3.0
+        uses: actions/setup-go@v4
         with:
           go-version-file: go.mod
+          check-latest: true
       - name: Run linters
         run: make lint

.golangci.yaml

@@ -15,16 +15,13 @@ issues:
 linters:
   enable:
     - bodyclose
-    - deadcode
     - errcheck
     - gosimple
     - govet
     - ineffassign
     - staticcheck
-    - structcheck
     - typecheck
     - unused
-    - varcheck
     #- depguard
     - revive
     - goimports
@@ -34,3 +31,9 @@ linters:
     - gocritic
     - prealloc
     #- gosec
+linters-settings:
+  revive:
+    ignore-generated-header: true
+    rules:
+      - name: unused-parameter
+        disabled: true

Dockerfile.reva

@@ -16,7 +16,7 @@
 # granted to it by virtue of its status as an Intergovernmental Organization
 # or submit itself to any jurisdiction.
 
-FROM golang:alpine3.16 as builder
+FROM golang:1.21-alpine3.18 as builder
 
 RUN apk --no-cache add \
   ca-certificates \

Dockerfile.revad

@@ -16,7 +16,7 @@
 # granted to it by virtue of its status as an Intergovernmental Organization
 # or submit itself to any jurisdiction.
 
-FROM golang:alpine3.16 as builder
+FROM golang:1.21-alpine3.18 as builder
 
 RUN apk --no-cache add \
   ca-certificates \
@@ -36,7 +36,7 @@ RUN make build-revad-docker && \
 
 RUN mkdir -p /etc/revad/ && echo "" > /etc/revad/revad.toml
 
-FROM golang:alpine3.16
+FROM golang:1.21-alpine3.18
 
 RUN apk --no-cache add \
   mailcap

Dockerfile.revad-eos

@@ -16,7 +16,7 @@
 # granted to it by virtue of its status as an Intergovernmental Organization
 # or submit itself to any jurisdiction.
 
-FROM golang:alpine3.16 as builder
+FROM golang:1.21-alpine3.18 as builder
 
 RUN apk --no-cache add \
   bash \

Makefile

@@ -30,7 +30,7 @@ toolchain-clean:
 
 $(GOLANGCI_LINT):
 	@mkdir -p $(@D)
-	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | BINDIR=$(@D) sh -s v1.51.2
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | BINDIR=$(@D) sh -s v1.55.2
 
 .PHONY: check-changelog
 lint: $(GOLANGCI_LINT)

changelog/unreleased/add-update-share-permission-check.md

@@ -0,0 +1,5 @@
+Enhancement: Check permissions before updating shares
+
+The user share provider now checks if the user has sufficient permissions to update a share.
+
+https://github.com/cs3org/reva/pull/4405

go.mod

@@ -61,7 +61,7 @@ require (
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/ginkgo/v2 v2.11.0
 	github.com/onsi/gomega v1.27.8
-	github.com/owncloud/ocis/v2 v2.0.1-0.20230606150602-25d7dae4667b
+	github.com/owncloud/ocis/v2 v2.0.0
 	github.com/pkg/errors v0.9.1
 	github.com/pkg/xattr v0.4.9
 	github.com/prometheus/alertmanager v0.24.0

go.sum

@@ -567,8 +567,8 @@ github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGE
 github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
 github.com/fatih/color v1.14.1 h1:qfhVLaG5s+nCROl1zJsZRxFeYrHLqWroPOQ8BWiNb4w=
 github.com/fatih/color v1.14.1/go.mod h1:2oHN61fhTpgcxD3TSWCgKDiH1+x4OiDVVGH8WlgGZGg=
-github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
-github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
+github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ=
+github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
 github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
@@ -707,8 +707,8 @@ github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x
 github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
 github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
 github.com/gofrs/uuid v4.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
-github.com/gofrs/uuid v4.4.0+incompatible h1:3qXRTX8/NbyulANqlc0lchS1gqAVxRgsuW1YrTJupqA=
-github.com/gofrs/uuid v4.4.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
+github.com/gofrs/uuid v4.3.1+incompatible h1:0/KbAdpx3UXAx1kEOWHJeOkpbgRFGHVgv+CFIY7dBJI=
+github.com/gofrs/uuid v4.3.1+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
 github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
@@ -840,9 +840,8 @@ github.com/hashicorp/consul/api v1.15.2 h1:3Q/pDqvJ7udgt/60QOOW/p/PeKioQN+ncYzzC
 github.com/hashicorp/consul/api v1.15.2/go.mod h1:v6nvB10borjOuIwNRZYPZiHKrTM/AyrGtd0WVVodKM8=
 github.com/hashicorp/consul/sdk v0.11.0 h1:HRzj8YSCln2yGgCumN5CL8lYlD3gBurnervJRJAZyC4=
 github.com/hashicorp/consul/sdk v0.11.0/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw=
+github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
-github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
-github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
@@ -1082,8 +1081,8 @@ github.com/onsi/gomega v1.27.8 h1:gegWiwZjBsf2DgiSbf5hpokZ98JVDMcWkUiigk6/KXc=
 github.com/onsi/gomega v1.27.8/go.mod h1:2J8vzI/s+2shY9XHRApDkdgPo1TKT7P2u6fXeJKFnNQ=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
 github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
-github.com/owncloud/ocis/v2 v2.0.1-0.20230606150602-25d7dae4667b h1:Aiou+DcU5B10HSCIumhg9X+2Qaljt6Qc+aIEfM5VsCc=
-github.com/owncloud/ocis/v2 v2.0.1-0.20230606150602-25d7dae4667b/go.mod h1:B+5L2ssUvYoy1yDi2PM7/qRAtsHcJ6SGGqC5SvmljjY=
+github.com/owncloud/ocis/v2 v2.0.0 h1:eHmUpW73dAT0X+JXRStYRzHt9gBUGlysnLg3vjJzacs=
+github.com/owncloud/ocis/v2 v2.0.0/go.mod h1:qH016gkfh/PNOv+xfiwD2weWY99nZTTghKhgajshYYk=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=

internal/grpc/services/gateway/usershareprovider.go

@@ -119,6 +119,9 @@ func (s *svc) updateShare(ctx context.Context, req *collaboration.UpdateShareReq
 	if err != nil {
 		return nil, errors.Wrap(err, "gateway: error calling UpdateShare")
 	}
+	if res.GetStatus().GetCode() != rpc.Code_CODE_OK {
+		return res, nil
+	}
 
 	if s.c.CommitShareToStorageGrant {
 		creator := ctxpkg.ContextMustGetUser(ctx)

internal/grpc/services/usershareprovider/usershareprovider.go

@@ -21,7 +21,9 @@ package usershareprovider
 import (
 	"context"
 	"regexp"
+	"slices"
 
+	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
@@ -34,20 +36,24 @@ import (
 	"github.com/cs3org/reva/v2/pkg/conversions"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
+	"github.com/cs3org/reva/v2/pkg/permission"
 	"github.com/cs3org/reva/v2/pkg/rgrpc"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/status"
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/share"
 	"github.com/cs3org/reva/v2/pkg/share/manager/registry"
+	"github.com/cs3org/reva/v2/pkg/sharedconf"
 	"github.com/cs3org/reva/v2/pkg/utils"
 )
 
 func init() {
-	rgrpc.Register("usershareprovider", New)
+	rgrpc.Register("usershareprovider", NewDefault)
 }
 
 type config struct {
 	Driver                string                            `mapstructure:"driver"`
 	Drivers               map[string]map[string]interface{} `mapstructure:"drivers"`
+	GatewayAddr           string                            `mapstructure:"gateway_addr"`
 	AllowedPathsForShares []string                          `mapstructure:"allowed_paths_for_shares"`
 }
 
@@ -58,8 +64,8 @@ func (c *config) init() {
 }
 
 type service struct {
-	conf                  *config
 	sm                    share.Manager
+	gatewaySelector       pool.Selectable[gateway.GatewayAPIClient]
 	allowedPathsForShares []*regexp.Regexp
 }
 
@@ -92,8 +98,8 @@ func parseConfig(m map[string]interface{}) (*config, error) {
 	return c, nil
 }
 
-// New creates a new user share provider svc
-func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
+// New creates a new user share provider svc initialized from defaults
+func NewDefault(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
 
 	c, err := parseConfig(m)
 	if err != nil {
@@ -116,13 +122,23 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
 		allowedPathsForShares = append(allowedPathsForShares, regex)
 	}
 
+	gatewaySelector, err := pool.GatewaySelector(sharedconf.GetGatewaySVC(c.GatewayAddr))
+	if err != nil {
+		return nil, err
+	}
+
+	return New(gatewaySelector, sm, allowedPathsForShares), nil
+}
+
+// New creates a new user share provider svc
+func New(gatewaySelector pool.Selectable[gateway.GatewayAPIClient], sm share.Manager, allowedPathsForShares []*regexp.Regexp) rgrpc.Service {
 	service := &service{
-		conf:                  c,
 		sm:                    sm,
+		gatewaySelector:       gatewaySelector,
 		allowedPathsForShares: allowedPathsForShares,
 	}
 
-	return service, nil
+	return service
 }
 
 func (s *service) isPathAllowed(path string) bool {
@@ -140,6 +156,24 @@ func (s *service) isPathAllowed(path string) bool {
 func (s *service) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) {
 	user := ctxpkg.ContextMustGetUser(ctx)
 
+	gatewayClient, err := s.gatewaySelector.Next()
+	if err != nil {
+		return nil, err
+	}
+
+	// check if the user has the permission to create shares at all
+	ok, err := utils.CheckPermission(ctx, permission.WriteShare, gatewayClient)
+	if err != nil {
+		return &collaboration.CreateShareResponse{
+			Status: status.NewInternal(ctx, "failed check user permission to write public link"),
+		}, err
+	}
+	if !ok {
+		return &collaboration.CreateShareResponse{
+			Status: status.NewPermissionDenied(ctx, nil, "no permission to create public links"),
+		}, nil
+	}
+
 	if req.GetGrant().GetGrantee().GetType() == provider.GranteeType_GRANTEE_TYPE_USER && req.GetGrant().GetGrantee().GetUserId().GetIdp() == "" {
 		// use logged in user Idp as default.
 		req.GetGrant().GetGrantee().Id = &provider.Grantee_UserId{
@@ -244,6 +278,78 @@ func (s *service) ListShares(ctx context.Context, req *collaboration.ListSharesR
 }
 
 func (s *service) UpdateShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
+	log := appctx.GetLogger(ctx)
+	gatewayClient, err := s.gatewaySelector.Next()
+	if err != nil {
+		return nil, err
+	}
+
+	// check if the user has the permission to create shares at all
+	ok, err := utils.CheckPermission(ctx, permission.WriteShare, gatewayClient)
+	if err != nil {
+		return &collaboration.UpdateShareResponse{
+			Status: status.NewInternal(ctx, "failed check user permission to write share"),
+		}, err
+	}
+	if !ok {
+		return &collaboration.UpdateShareResponse{
+			Status: status.NewPermissionDenied(ctx, nil, "no permission to create user share"),
+		}, nil
+	}
+
+	// Read share from backend. We need the shared resource's id for STATing it, it might not be in
+	// the incoming request
+	currentShare, err := s.sm.GetShare(ctx,
+		&collaboration.ShareReference{
+			Spec: &collaboration.ShareReference_Id{
+				Id: req.GetShare().GetId(),
+			},
+		},
+	)
+	if err != nil {
+		var st *rpc.Status
+		switch err.(type) {
+		case errtypes.IsNotFound:
+			st = status.NewNotFound(ctx, err.Error())
+		default:
+			st = status.NewInternal(ctx, err.Error())
+		}
+		return &collaboration.UpdateShareResponse{
+			Status: st,
+		}, nil
+	}
+
+	sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: currentShare.GetResourceId()}})
+	if err != nil {
+		log.Err(err).Interface("resource_id", req.GetShare().GetResourceId()).Msg("failed to stat resource to share")
+		return &collaboration.UpdateShareResponse{
+			Status: status.NewInternal(ctx, "failed to stat shared resource"),
+		}, err
+	}
+
+	// If this is a permissions update, check if user's permissions on the resource are sufficient to set the desired permissions
+	var newPermissions *provider.ResourcePermissions
+	if slices.Contains(req.GetUpdateMask().GetPaths(), "permissions") {
+		newPermissions = req.GetShare().GetPermissions().GetPermissions()
+	} else {
+		newPermissions = req.GetField().GetPermissions().GetPermissions()
+	}
+	if newPermissions != nil && !conversions.SufficientCS3Permissions(sRes.GetInfo().GetPermissionSet(), newPermissions) {
+		return &collaboration.UpdateShareResponse{
+			Status: status.NewPermissionDenied(ctx, nil, "insufficient permissions to create that kind of share"),
+		}, nil
+	}
+
+	// check if the requested permission are plausible for the Resource
+	// do we need more here?
+	if sRes.GetInfo().GetType() == provider.ResourceType_RESOURCE_TYPE_FILE {
+		if newPermissions.GetCreateContainer() || newPermissions.GetMove() || newPermissions.GetDelete() {
+			return &collaboration.UpdateShareResponse{
+				Status: status.NewInvalid(ctx, "cannot set the requested permissions on that type of resource"),
+			}, nil
+		}
+	}
+
 	share, err := s.sm.UpdateShare(ctx, req.Ref, req.Field.GetPermissions(), req.Share, req.UpdateMask) // TODO(labkode): check what to update
 	if err != nil {
 		return &collaboration.UpdateShareResponse{