From c09d8e8c7c136472b77208a709a2e8736174286c Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 1 Mar 2022 08:29:05 +0100 Subject: [PATCH] use shares api for space members --- .../2.0.0_2022-03-03/use-share-api-spaces.md | 5 + changelog/unreleased/use-share-api-spaces.md | 5 + .../grpc/services/gateway/ocmshareprovider.go | 2 +- .../services/gateway/usershareprovider.go | 342 ++++++++++++------ .../handlers/apps/sharing/shares/spaces.go | 55 ++- 5 files changed, 266 insertions(+), 143 deletions(-) create mode 100644 changelog/2.0.0_2022-03-03/use-share-api-spaces.md create mode 100644 changelog/unreleased/use-share-api-spaces.md diff --git a/changelog/2.0.0_2022-03-03/use-share-api-spaces.md b/changelog/2.0.0_2022-03-03/use-share-api-spaces.md new file mode 100644 index 00000000000..401fa4acbb5 --- /dev/null +++ b/changelog/2.0.0_2022-03-03/use-share-api-spaces.md @@ -0,0 +1,5 @@ +Change: Use the cs3 share api to manage spaces + +We now use the cs3 share Api to manage the space roles. We do not send the request to the share manager, the permissions are stored in the storage provider + +https://github.com/cs3org/reva/pull/2600 diff --git a/changelog/unreleased/use-share-api-spaces.md b/changelog/unreleased/use-share-api-spaces.md new file mode 100644 index 00000000000..1f2a7ecfcb3 --- /dev/null +++ b/changelog/unreleased/use-share-api-spaces.md @@ -0,0 +1,5 @@ +Change: Use the cs3 Sharing api for spaces + +Space membership operations are now using the CS3 Sharing API. + +https://github.com/cs3org/reva/pull/2600 diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go index 0c146ac01c9..5f0dc925e92 100644 --- a/internal/grpc/services/gateway/ocmshareprovider.go +++ b/internal/grpc/services/gateway/ocmshareprovider.go @@ -54,7 +54,7 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest // TODO(labkode): if both commits are enabled they could be done concurrently. if s.c.CommitShareToStorageGrant { - addGrantStatus, err := s.addGrant(ctx, req.ResourceId, req.Grant.Grantee, req.Grant.Permissions.Permissions) + addGrantStatus, err := s.addGrant(ctx, req.ResourceId, req.Grant.Grantee, req.Grant.Permissions.Permissions, nil) if err != nil { return nil, errors.Wrap(err, "gateway: error adding OCM grant to storage") } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 1965289f5a3..4283e9ae824 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -38,122 +38,20 @@ import ( // TODO(labkode): add multi-phase commit logic when commit share or commit ref is enabled. func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { - c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint) - if err != nil { - appctx.GetLogger(ctx). - Err(err). - Msg("CreateShare: failed to get user share provider") - return &collaboration.CreateShareResponse{ - Status: status.NewInternal(ctx, "error getting user share provider client"), - }, nil - } - - // TODO the user share manager needs to be able to decide if the current user is allowed to create that share (and not eg. incerase permissions) - // jfd: AFAICT this can only be determined by a storage driver - either the storage provider is queried first or the share manager needs to access the storage using a storage driver - res, err := c.CreateShare(ctx, req) - if err != nil { - return nil, errors.Wrap(err, "gateway: error calling CreateShare") - } - if res.Status.Code != rpc.Code_CODE_OK { - return res, nil - } - - // if we don't need to commit we return earlier - if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { - return res, nil - } - - // TODO(labkode): if both commits are enabled they could be done concurrently. - if s.c.CommitShareToStorageGrant { - // If the share is a denial we call denyGrant instead. - var status *rpc.Status - if grants.PermissionsEqual(req.Grant.Permissions.Permissions, &provider.ResourcePermissions{}) { - status, err = s.denyGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee) - if err != nil { - return nil, errors.Wrap(err, "gateway: error denying grant in storage") - } - } else { - status, err = s.addGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, req.Grant.Permissions.Permissions) - if err != nil { - return nil, errors.Wrap(err, "gateway: error adding grant to storage") - } - } + isSpaceRoot := req.ResourceInfo.Id.StorageId == req.ResourceInfo.Id.OpaqueId - switch status.Code { - case rpc.Code_CODE_OK: - // ok - case rpc.Code_CODE_UNIMPLEMENTED: - appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants not supported, ignoring") - default: - return &collaboration.CreateShareResponse{ - Status: status, - }, err - } + // Don't use the share manager when sharing a space root + if isSpaceRoot { + return s.addSpaceShare(ctx, req) } - - s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.ResourceInfo.Id) - return res, nil + return s.addShare(ctx, req) } func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareRequest) (*collaboration.RemoveShareResponse, error) { - c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint) - if err != nil { - appctx.GetLogger(ctx). - Err(err). - Msg("RemoveShare: failed to get user share provider") - return &collaboration.RemoveShareResponse{ - Status: status.NewInternal(ctx, "error getting user share provider client"), - }, nil + if key := req.Ref.GetKey(); key != nil && key.ResourceId.StorageId == key.ResourceId.OpaqueId { + return s.removeSpaceShare(ctx, key.ResourceId, key.Grantee) } - - // if we need to commit the share, we need the resource it points to. - var share *collaboration.Share - if s.c.CommitShareToStorageGrant || s.c.CommitShareToStorageRef { - getShareReq := &collaboration.GetShareRequest{ - Ref: req.Ref, - } - getShareRes, err := c.GetShare(ctx, getShareReq) - if err != nil { - return nil, errors.Wrap(err, "gateway: error calling GetShare") - } - - if getShareRes.Status.Code != rpc.Code_CODE_OK { - res := &collaboration.RemoveShareResponse{ - Status: status.NewInternal(ctx, - "error getting share when committing to the storage"), - } - return res, nil - } - share = getShareRes.Share - } - - res, err := c.RemoveShare(ctx, req) - if err != nil { - return nil, errors.Wrap(err, "gateway: error calling RemoveShare") - } - - s.removeReference(ctx, share.ResourceId) - - // if we don't need to commit we return earlier - if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { - return res, nil - } - - // TODO(labkode): if both commits are enabled they could be done concurrently. - if s.c.CommitShareToStorageGrant { - removeGrantStatus, err := s.removeGrant(ctx, share.ResourceId, share.Grantee, share.Permissions.Permissions) - if err != nil { - return nil, errors.Wrap(err, "gateway: error removing grant from storage") - } - if removeGrantStatus.Code != rpc.Code_CODE_OK { - return &collaboration.RemoveShareResponse{ - Status: removeGrantStatus, - }, err - } - } - - s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), share.ResourceId) - return res, nil + return s.removeShare(ctx, req) } // TODO(labkode): we need to validate share state vs storage grant and storage ref @@ -212,7 +110,6 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq Status: status.NewInternal(ctx, "error getting share provider client"), }, nil } - res, err := c.UpdateShare(ctx, req) if err != nil { return nil, errors.Wrap(err, "gateway: error calling UpdateShare") @@ -468,7 +365,7 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource return status.NewOK(ctx) } -func (s *svc) denyGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee) (*rpc.Status, error) { +func (s *svc) denyGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, opaque *typesv1beta1.Opaque) (*rpc.Status, error) { ref := &provider.Reference{ ResourceId: id, } @@ -476,6 +373,7 @@ func (s *svc) denyGrant(ctx context.Context, id *provider.ResourceId, g *provide grantReq := &provider.DenyGrantRequest{ Ref: ref, Grantee: g, + Opaque: opaque, } c, _, err := s.find(ctx, ref) @@ -497,7 +395,7 @@ func (s *svc) denyGrant(ctx context.Context, id *provider.ResourceId, g *provide return grantRes.Status, nil } -func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions) (*rpc.Status, error) { +func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider.Grantee, p *provider.ResourcePermissions, opaque *typesv1beta1.Opaque) (*rpc.Status, error) { ref := &provider.Reference{ ResourceId: id, } @@ -508,6 +406,7 @@ func (s *svc) addGrant(ctx context.Context, id *provider.ResourceId, g *provider Grantee: g, Permissions: p, }, + Opaque: opaque, } c, _, err := s.find(ctx, ref) @@ -601,3 +500,220 @@ func (s *svc) removeGrant(ctx context.Context, id *provider.ResourceId, g *provi return status.NewOK(ctx), nil } + +func (s *svc) listGrants(ctx context.Context, id *provider.ResourceId) (*provider.ListGrantsResponse, error) { + ref := &provider.Reference{ + ResourceId: id, + } + + grantReq := &provider.ListGrantsRequest{ + Ref: ref, + } + + c, _, err := s.find(ctx, ref) + if err != nil { + appctx.GetLogger(ctx). + Err(err). + Interface("reference", ref). + Msg("removeGrant: failed to get storage provider") + if _, ok := err.(errtypes.IsNotFound); ok { + return &provider.ListGrantsResponse{ + Status: status.NewNotFound(ctx, "storage provider not found"), + }, nil + } + return &provider.ListGrantsResponse{ + Status: status.NewInternal(ctx, "error finding storage provider"), + }, nil + } + + grantRes, err := c.ListGrants(ctx, grantReq) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling ListGrants") + } + if grantRes.Status.Code != rpc.Code_CODE_OK { + return &provider.ListGrantsResponse{Status: status.NewInternal(ctx, + "error listing storage grants"), + }, + nil + } + return grantRes, nil +} + +func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { + c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint) + if err != nil { + appctx.GetLogger(ctx). + Err(err). + Msg("CreateShare: failed to get user share provider") + return &collaboration.CreateShareResponse{ + Status: status.NewInternal(ctx, "error getting user share provider client"), + }, nil + } + // TODO the user share manager needs to be able to decide if the current user is allowed to create that share (and not eg. incerase permissions) + // jfd: AFAICT this can only be determined by a storage driver - either the storage provider is queried first or the share manager needs to access the storage using a storage driver + res, err := c.CreateShare(ctx, req) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling CreateShare") + } + if res.Status.Code != rpc.Code_CODE_OK { + return res, nil + } + // if we don't need to commit we return earlier + if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { + return res, nil + } + + // TODO(labkode): if both commits are enabled they could be done concurrently. + if s.c.CommitShareToStorageGrant { + // If the share is a denial we call denyGrant instead. + var status *rpc.Status + if grants.PermissionsEqual(req.Grant.Permissions.Permissions, &provider.ResourcePermissions{}) { + status, err = s.denyGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, nil) + if err != nil { + return nil, errors.Wrap(err, "gateway: error denying grant in storage") + } + } else { + status, err = s.addGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, req.Grant.Permissions.Permissions, nil) + if err != nil { + return nil, errors.Wrap(err, "gateway: error adding grant to storage") + } + } + + switch status.Code { + case rpc.Code_CODE_OK: + s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.ResourceInfo.Id) + case rpc.Code_CODE_UNIMPLEMENTED: + appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants not supported, ignoring") + default: + return &collaboration.CreateShareResponse{ + Status: status, + }, err + } + } + return res, nil +} + +func (s *svc) addSpaceShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, 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": {}, + }, + } + if grants.PermissionsEqual(req.Grant.Permissions.Permissions, &provider.ResourcePermissions{}) { + st, err = s.denyGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, &opaque) + if err != nil { + return nil, errors.Wrap(err, "gateway: error denying grant in storage") + } + } else { + st, err = s.addGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, req.Grant.Permissions.Permissions, &opaque) + if err != nil { + return nil, errors.Wrap(err, "gateway: error adding grant to storage") + } + } + + switch st.Code { + case rpc.Code_CODE_OK: + s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), req.ResourceInfo.Id) + case rpc.Code_CODE_UNIMPLEMENTED: + appctx.GetLogger(ctx).Debug().Interface("status", st).Interface("req", req).Msg("storing grants not supported, ignoring") + default: + return &collaboration.CreateShareResponse{ + Status: st, + }, err + } + + return &collaboration.CreateShareResponse{ + Status: status.NewOK(ctx), + Share: &collaboration.Share{ + ResourceId: req.ResourceInfo.Id, + Permissions: &collaboration.SharePermissions{Permissions: req.Grant.Permissions.GetPermissions()}, + Grantee: req.Grant.Grantee, + }, + }, nil +} + +func (s *svc) removeShare(ctx context.Context, req *collaboration.RemoveShareRequest) (*collaboration.RemoveShareResponse, error) { + c, err := pool.GetUserShareProviderClient(s.c.UserShareProviderEndpoint) + if err != nil { + appctx.GetLogger(ctx). + Err(err). + Msg("RemoveShare: failed to get user share provider") + return &collaboration.RemoveShareResponse{ + Status: status.NewInternal(ctx, "error getting user share provider client"), + }, nil + } + + // if we need to commit the share, we need the resource it points to. + var share *collaboration.Share + if s.c.CommitShareToStorageGrant || s.c.CommitShareToStorageRef { + getShareReq := &collaboration.GetShareRequest{ + Ref: req.Ref, + } + getShareRes, err := c.GetShare(ctx, getShareReq) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling GetShare") + } + + if getShareRes.Status.Code != rpc.Code_CODE_OK { + res := &collaboration.RemoveShareResponse{ + Status: status.NewInternal(ctx, + "error getting share when committing to the storage"), + } + return res, nil + } + share = getShareRes.Share + } + + res, err := c.RemoveShare(ctx, req) + if err != nil { + return nil, errors.Wrap(err, "gateway: error calling RemoveShare") + } + + s.removeReference(ctx, share.ResourceId) + + // if we don't need to commit we return earlier + if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { + return res, nil + } + + // TODO(labkode): if both commits are enabled they could be done concurrently. + if s.c.CommitShareToStorageGrant { + removeGrantStatus, err := s.removeGrant(ctx, share.ResourceId, share.Grantee, share.Permissions.Permissions) + if err != nil { + return nil, errors.Wrap(err, "gateway: error removing grant from storage") + } + if removeGrantStatus.Code != rpc.Code_CODE_OK { + return &collaboration.RemoveShareResponse{ + Status: removeGrantStatus, + }, err + } + } + + s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), share.ResourceId) + return res, nil +} + +func (s *svc) removeSpaceShare(ctx context.Context, ref *provider.ResourceId, grantee *provider.Grantee) (*collaboration.RemoveShareResponse, error) { + // TODO(labkode): if both commits are enabled they could be done concurrently. + + listGrantRes, err := s.listGrants(ctx, ref) + if err != nil { + return nil, errors.Wrap(err, "gateway: error getting grant to remove from storage") + } + removeGrantStatus, err := s.removeGrant(ctx, ref, grantee, listGrantRes.Grants[0].Permissions) + if err != nil { + return nil, errors.Wrap(err, "gateway: error removing grant from storage") + } + if removeGrantStatus.Code != rpc.Code_CODE_OK { + return &collaboration.RemoveShareResponse{ + Status: removeGrantStatus, + }, err + } + s.cache.RemoveStat(ctxpkg.ContextMustGetUser(ctx), ref) + return &collaboration.RemoveShareResponse{Status: status.NewOK(ctx)}, nil + +} 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 45c352ced4b..60e332387b9 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 @@ -26,6 +26,7 @@ import ( groupv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + collaborationv1beta1 "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" @@ -88,36 +89,22 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p return } - ref := &provider.Reference{ResourceId: info.Id} - - 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) + client, err := pool.GetGatewayServiceClient(h.gatewayAddr) if err != nil { - response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider client", err) + response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting gateway client", err) return } - // TODO: change CS3 APIs - opaque := &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "spacegrant": {}, - }, - } - - addGrantRes, err := providerClient.AddGrant(ctx, &provider.AddGrantRequest{ - Opaque: opaque, - Ref: ref, - Grant: &provider.Grant{ - Grantee: &grantee, - Permissions: role.CS3ResourcePermissions(), + createShareRes, err := client.CreateShare(ctx, &collaborationv1beta1.CreateShareRequest{ + ResourceInfo: info, + Grant: &collaborationv1beta1.ShareGrant{ + Permissions: &collaborationv1beta1.SharePermissions{ + Permissions: role.CS3ResourcePermissions(), + }, + Grantee: &grantee, }, }) - if err != nil || addGrantRes.Status.Code != rpc.Code_CODE_OK { + if err != nil || createShareRes.Status.Code != rpc.Code_CODE_OK { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "could not add space member", err) return } @@ -169,17 +156,27 @@ func (h *Handler) removeSpaceMember(w http.ResponseWriter, r *http.Request, spac return } - removeGrantRes, err := providerClient.RemoveGrant(ctx, &provider.RemoveGrantRequest{ - Ref: &ref, - Grant: &provider.Grant{ - Grantee: &grantee, + gatewayClient, err := pool.GetGatewayServiceClient(h.gatewayAddr) + if err != nil { + response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting gateway client", err) + return + } + + removeShareRes, err := gatewayClient.RemoveShare(ctx, &collaborationv1beta1.RemoveShareRequest{ + Ref: &collaborationv1beta1.ShareReference{ + Spec: &collaborationv1beta1.ShareReference_Key{ + Key: &collaborationv1beta1.ShareKey{ + ResourceId: ref.ResourceId, + Grantee: &grantee, + }, + }, }, }) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error removing grant", err) return } - if removeGrantRes.Status.Code != rpc.Code_CODE_OK { + if removeShareRes.Status.Code != rpc.Code_CODE_OK { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error removing grant", err) return }