From c1d1d7b3c4e135c9451b0d26a12312baada6e1a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 18 Aug 2023 15:22:00 +0200 Subject: [PATCH] fix space grant updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../services/gateway/usershareprovider.go | 77 ++++++++++++++++--- .../handlers/apps/sharing/shares/spaces.go | 21 +++-- 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 07e50c39e07..bbcfbf0bc4a 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/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) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go index 03deb4759dc..a473ca0f43e 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go +++ b/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" ) @@ -150,21 +151,27 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p return } + // 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) { - updateShareRes, err := providerClient.UpdateGrant(ctx, &provider.UpdateGrantRequest{ + updateShareReq := &collaborationv1beta1.UpdateShareRequest{ // TODO: change CS3 APIs Opaque: &types.Opaque{ Map: map[string]*types.OpaqueEntry{ "spacegrant": {}, }, }, - Ref: &ref, - Grant: &provider.Grant{ - Permissions: permissions, - Grantee: &grantee, - Expiration: expirationTs, + 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