Grant locking

changelog/unreleased/grant-locking.md

@@ -0,0 +1,7 @@
+Bugfix: fix jsoncs3 atomic persistence
+
+The jsoncs3 share manager now uses etags instead of mtimes to determine when metadata needs to be updated.
+As a precondtition we had to change decomposedfs as well: to consistently calculate the etag for the file content
+we now store the mtime in the metadata and use the metadata lock for atomicity.
+
+https://github.com/cs3org/reva/pull/4117

go.mod

@@ -82,6 +82,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.16.0
 	go.opentelemetry.io/otel/trace v1.16.0
 	golang.org/x/crypto v0.11.0
+	golang.org/x/exp v0.0.0-20221026004748-78e5e7837ae6
 	golang.org/x/oauth2 v0.10.0
 	golang.org/x/sync v0.2.0
 	golang.org/x/sys v0.10.0

go.sum

@@ -1305,6 +1305,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20221026004748-78e5e7837ae6 h1:mC6uOkPi9SUk8A59jZvw7//rlyc+MlELtQUCyOUSKZQ=
+golang.org/x/exp v0.0.0-20221026004748-78e5e7837ae6/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=

internal/grpc/services/gateway/usershareprovider.go

@@ -52,6 +52,13 @@ func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareReq
 	return s.removeShare(ctx, req)
 }
 
+func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
+	if !s.c.UseCommonSpaceRootShareLogic && refIsSpaceRoot(req.GetShare().GetResourceId()) {
+		return s.updateSpaceShare(ctx, req)
+	}
+	return s.updateShare(ctx, req)
+}
+
 // TODO(labkode): we need to validate share state vs storage grant and storage ref
 // If there are any inconsistencies, the share needs to be flag as invalid and a background process
 // or active fix needs to be performed.
@@ -98,7 +105,7 @@ func (s *svc) ListShares(ctx context.Context, req *collaboration.ListSharesReque
 	return res, nil
 }
 
-func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
+func (s *svc) updateShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
 	c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint)
 	if err != nil {
 		appctx.GetLogger(ctx).
@@ -114,9 +121,14 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 	}
 
 	if s.c.CommitShareToStorageGrant {
-		updateGrantStatus, err := s.updateGrant(ctx, res.GetShare().GetResourceId(),
-			res.GetShare().GetGrantee(),
-			res.GetShare().GetPermissions().GetPermissions())
+		creator := ctxpkg.ContextMustGetUser(ctx)
+		grant := &provider.Grant{
+			Grantee:     req.GetShare().GetGrantee(),
+			Permissions: req.GetShare().GetPermissions().GetPermissions(),
+			Expiration:  req.GetShare().GetExpiration(),
+			Creator:     creator.GetId(),
+		}
+		updateGrantStatus, err := s.updateGrant(ctx, res.GetShare().GetResourceId(), grant, nil)
 
 		if err != nil {
 			return nil, errors.Wrap(err, "gateway: error calling updateGrant")
@@ -134,6 +146,51 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 	return res, nil
 }
 
+func (s *svc) updateSpaceShare(ctx context.Context, req *collaboration.UpdateShareRequest) (*collaboration.UpdateShareResponse, error) {
+	// If the share is a denial we call  denyGrant instead.
+	var st *rpc.Status
+	var err error
+	// TODO: change CS3 APIs
+	opaque := &typesv1beta1.Opaque{
+		Map: map[string]*typesv1beta1.OpaqueEntry{
+			"spacegrant": {},
+		},
+	}
+	utils.AppendPlainToOpaque(opaque, "spacetype", utils.ReadPlainFromOpaque(req.Opaque, "spacetype"))
+
+	creator := ctxpkg.ContextMustGetUser(ctx)
+	grant := &provider.Grant{
+		Grantee:     req.GetShare().GetGrantee(),
+		Permissions: req.GetShare().GetPermissions().GetPermissions(),
+		Expiration:  req.GetShare().GetExpiration(),
+		Creator:     creator.GetId(),
+	}
+	if grants.PermissionsEqual(req.Share.GetPermissions().GetPermissions(), &provider.ResourcePermissions{}) {
+		st, err = s.denyGrant(ctx, req.GetShare().GetResourceId(), req.GetShare().GetGrantee(), opaque)
+		if err != nil {
+			return nil, errors.Wrap(err, "gateway: error denying grant in storage")
+		}
+	} else {
+		st, err = s.updateGrant(ctx, req.GetShare().GetResourceId(), grant, opaque)
+		if err != nil {
+			return nil, errors.Wrap(err, "gateway: error adding grant to storage")
+		}
+	}
+
+	res := &collaboration.UpdateShareResponse{
+		Status: st,
+		Share:  req.Share,
+	}
+
+	if st.Code != rpc.Code_CODE_OK {
+		return res, nil
+	}
+
+	s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.GetShare().GetResourceId())
+	s.providerCache.RemoveListStorageProviders(req.GetShare().GetResourceId())
+	return res, nil
+}
+
 // TODO(labkode): listing received shares just goes to the user share manager and gets the list of
 // received shares. The display name of the shares should be the a friendly name, like the basename
 // of the original file.
@@ -333,19 +390,15 @@ func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider
 	return grantRes.Status, nil
 }
 
-func (s *svc) updateGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions) (*rpc.Status, error) {
+func (s *svc) updateGrant(ctx context.Context, id *provider.ResourceId, grant *provider.Grant, opaque *typesv1beta1.Opaque) (*rpc.Status, error) {
 	ref := &provider.Reference{
 		ResourceId: id,
 	}
 
-	creator := ctxpkg.ContextMustGetUser(ctx)
 	grantReq := &provider.UpdateGrantRequest{
-		Ref: ref,
-		Grant: &provider.Grant{
-			Grantee:     g,
-			Permissions: p,
-			Creator:     creator.GetId(),
-		},
+		Opaque: opaque,
+		Ref:    ref,
+		Grant:  grant,
 	}
 
 	c, _, err := s.find(ctx, ref)

internal/grpc/services/storageprovider/storageprovider.go

@@ -324,7 +324,13 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate
 		return nil, err
 	}
 	switch sRes.Status.Code {
-	case rpc.Code_CODE_OK, rpc.Code_CODE_NOT_FOUND:
+	case rpc.Code_CODE_OK:
+		if req.GetIfNotExist() {
+			return &provider.InitiateFileUploadResponse{
+				Status: status.NewAlreadyExists(ctx, errors.New("already exists"), "already exists"),
+			}, nil
+		}
+	case rpc.Code_CODE_NOT_FOUND:
 		// Just continue with a normal upload
 	default:
 		return &provider.InitiateFileUploadResponse{
@@ -342,10 +348,14 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate
 		}
 		metadata["if-match"] = ifMatch
 	}
-	if !validateIfUnmodifiedSince(req.GetIfUnmodifiedSince(), sRes.GetInfo()) {
-		return &provider.InitiateFileUploadResponse{
-			Status: status.NewFailedPrecondition(ctx, errors.New("resource has been modified"), "resource has been modified"),
-		}, nil
+	ifUnmodifiedSince := req.GetIfUnmodifiedSince()
+	if ifUnmodifiedSince != nil {
+		metadata["if-unmodified-since"] = utils.TSToTime(ifUnmodifiedSince).Format(time.RFC3339Nano)
+		if !validateIfUnmodifiedSince(ifUnmodifiedSince, sRes.GetInfo()) {
+			return &provider.InitiateFileUploadResponse{
+				Status: status.NewFailedPrecondition(ctx, errors.New("resource has been modified"), "resource has been modified"),
+			}, nil
+		}
 	}
 
 	ctx = ctxpkg.ContextSetLockID(ctx, req.LockId)
@@ -1080,6 +1090,19 @@ func (s *service) UpdateGrant(ctx context.Context, req *provider.UpdateGrantRequ
 		}
 	}
 
+	// TODO: update CS3 APIs
+	// FIXME these should be part of the AddGrantRequest object
+	// https://github.com/owncloud/ocis/issues/4312
+	if utils.ExistsInOpaque(req.Opaque, "spacegrant") {
+		ctx = context.WithValue(
+			ctx,
+			utils.SpaceGrant,
+			struct{ SpaceType string }{
+				SpaceType: utils.ReadPlainFromOpaque(req.Opaque, "spacetype"),
+			},
+		)
+	}
+
 	// check grantee type is valid
 	if req.Grant.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_INVALID {
 		return &provider.UpdateGrantResponse{

internal/http/services/owncloud/ocdav/net/headers.go

@@ -33,6 +33,7 @@ const (
 	HeaderLocation                   = "Location"
 	HeaderRange                      = "Range"
 	HeaderIfMatch                    = "If-Match"
+	HeaderIfNoneMatch                = "If-None-Match"
 	HeaderPrefer                     = "Prefer"
 	HeaderPreferenceApplied          = "Preference-Applied"
 	HeaderVary                       = "Vary"

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

@@ -39,6 +39,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	sdk "github.com/cs3org/reva/v2/pkg/sdk/common"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/pkg/errors"
 )
 
@@ -126,45 +127,70 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p
 		}
 	}
 
-	if role.Name != conversions.RoleManager {
-		ref := provider.Reference{ResourceId: info.GetId()}
-		p, err := h.findProvider(ctx, &ref)
-		if err != nil {
-			response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider", err)
-			return
-		}
+	ref := provider.Reference{ResourceId: info.GetId()}
+	p, err := h.findProvider(ctx, &ref)
+	if err != nil {
+		response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider", err)
+		return
+	}
 
-		providerClient, err := h.getStorageProviderClient(p)
-		if err != nil {
-			response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider client", err)
-			return
-		}
+	providerClient, err := h.getStorageProviderClient(p)
+	if err != nil {
+		response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider client", err)
+		return
+	}
 
-		lgRes, err := providerClient.ListGrants(ctx, &provider.ListGrantsRequest{Ref: &ref})
-		if err != nil || lgRes.Status.Code != rpc.Code_CODE_OK {
-			response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error listing space grants", err)
-			return
-		}
+	lgRes, err := providerClient.ListGrants(ctx, &provider.ListGrantsRequest{Ref: &ref})
+	if err != nil || lgRes.Status.Code != rpc.Code_CODE_OK {
+		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error listing space grants", err)
+		return
+	}
 
-		if !isSpaceManagerRemaining(lgRes.Grants, grantee) {
-			response.WriteOCSError(w, r, http.StatusForbidden, "the space must have at least one manager", nil)
-			return
-		}
+	if !isSpaceManagerRemaining(lgRes.Grants, grantee) {
+		response.WriteOCSError(w, r, http.StatusForbidden, "the space must have at least one manager", nil)
+		return
 	}
 
-	createShareRes, err := client.CreateShare(ctx, &collaborationv1beta1.CreateShareRequest{
-		ResourceInfo: info,
-		Grant: &collaborationv1beta1.ShareGrant{
-			Permissions: &collaborationv1beta1.SharePermissions{
-				Permissions: permissions,
+	// we have to send the update request to the gateway to give it a chance to invalidate its cache
+	// TODO the gateway no longer should cache stuff because invalidation is to expensive. The decomposedfs already has a better cache.
+	if granteeExists(lgRes.Grants, grantee) {
+		updateShareReq := &collaborationv1beta1.UpdateShareRequest{
+			// TODO: change CS3 APIs
+			Opaque: &types.Opaque{
+				Map: map[string]*types.OpaqueEntry{
+					"spacegrant": {},
+				},
 			},
-			Grantee:    &grantee,
-			Expiration: expirationTs,
-		},
-	})
-	if err != nil || createShareRes.Status.Code != rpc.Code_CODE_OK {
-		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not add space member", err)
-		return
+			Share: &collaborationv1beta1.Share{
+				ResourceId: ref.GetResourceId(),
+				Permissions: &collaborationv1beta1.SharePermissions{
+					Permissions: permissions,
+				},
+				Grantee:    &grantee,
+				Expiration: expirationTs,
+			},
+		}
+		updateShareReq.Opaque = utils.AppendPlainToOpaque(updateShareReq.Opaque, "spacetype", info.GetSpace().GetSpaceType())
+		updateShareRes, err := client.UpdateShare(ctx, updateShareReq)
+		if err != nil || updateShareRes.Status.Code != rpc.Code_CODE_OK {
+			response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not update space member grant", err)
+			return
+		}
+	} else {
+		createShareRes, err := client.CreateShare(ctx, &collaborationv1beta1.CreateShareRequest{
+			ResourceInfo: info,
+			Grant: &collaborationv1beta1.ShareGrant{
+				Permissions: &collaborationv1beta1.SharePermissions{
+					Permissions: permissions,
+				},
+				Grantee:    &grantee,
+				Expiration: expirationTs,
+			},
+		})
+		if err != nil || createShareRes.Status.Code != rpc.Code_CODE_OK {
+			response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not add space member grant", err)
+			return
+		}
 	}
 
 	response.WriteOCSSuccess(w, r, nil)
@@ -325,6 +351,15 @@ func isSpaceManagerRemaining(grants []*provider.Grant, grantee provider.Grantee)
 	return false
 }
 
+func granteeExists(grants []*provider.Grant, grantee provider.Grantee) bool {
+	for _, g := range grants {
+		if isEqualGrantee(*g.Grantee, grantee) {
+			return true
+		}
+	}
+	return false
+}
+
 func isEqualGrantee(a, b provider.Grantee) bool {
 	// Ideally we would want to use utils.GranteeEqual()
 	// but the grants stored in the decomposedfs aren't complete (missing usertype and idp)

pkg/errtypes/errtypes.go

@@ -23,6 +23,7 @@
 package errtypes
 
 import (
+	"net/http"
 	"strings"
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -172,6 +173,19 @@ func (e InsufficientStorage) StatusCode() int {
 	return StatusInsufficientStorage
 }
 
+// NotModified is the error to use when a resource was not modified, e.g. the requested etag did not change.
+type NotModified string
+
+func (e NotModified) Error() string { return "error: not modified: " + string(e) }
+
+// NotModified implements the IsNotModified interface.
+func (e NotModified) IsNotModified() {}
+
+// StatusCode returns StatusInsufficientStorage, this implementation is needed to allow TUS to cast the correct http errors.
+func (e NotModified) StatusCode() int {
+	return http.StatusNotModified
+}
+
 // Body returns the error body. This implementation is needed to allow TUS to cast the correct http errors
 func (e InsufficientStorage) Body() []byte {
 	return []byte(e.Error())
@@ -302,3 +316,37 @@ func NewErrtypeFromStatus(status *rpc.Status) error {
 		return InternalError(status.Message)
 	}
 }
+
+// NewErrtypeFromHTTPStatusCode maps an http status to an errtype
+func NewErrtypeFromHTTPStatusCode(code int, message string) error {
+	switch code {
+	case http.StatusOK:
+		return nil
+	case http.StatusNotFound:
+		return NotFound(message)
+	case http.StatusConflict:
+		return AlreadyExists(message)
+	case http.StatusNotImplemented:
+		return NotSupported(message)
+	case http.StatusNotModified:
+		return NotModified(message)
+	case http.StatusForbidden:
+		return PermissionDenied(message)
+	case http.StatusLocked:
+		return Locked(message)
+	case http.StatusPreconditionFailed:
+		return Aborted(message)
+	case http.StatusMethodNotAllowed:
+		return PreconditionFailed(message)
+	case http.StatusInsufficientStorage:
+		return InsufficientStorage(message)
+	case http.StatusBadRequest:
+		return BadRequest(message)
+	case http.StatusPartialContent:
+		return PartialContent(message)
+	case StatusChecksumMismatch:
+		return ChecksumMismatch(message)
+	default:
+		return InternalError(message)
+	}
+}