From 48f76f979303a020de348ac88f2d94cefba71b46 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Thu, 8 Sep 2022 12:12:22 +0200 Subject: [PATCH] enable space member to update share (#3192) * enable space member to update share * add permission checks Check the permissions of the user before returning or updating shares. * update jsoncs3 tests --- changelog/unreleased/update-share.md | 5 ++ pkg/share/manager/jsoncs3/jsoncs3.go | 78 ++++++++++++++++++----- pkg/share/manager/jsoncs3/jsoncs3_test.go | 56 +++++++++++++--- pkg/share/share.go | 7 ++ 4 files changed, 121 insertions(+), 25 deletions(-) create mode 100644 changelog/unreleased/update-share.md diff --git a/changelog/unreleased/update-share.md b/changelog/unreleased/update-share.md new file mode 100644 index 0000000000..f9071a2c2b --- /dev/null +++ b/changelog/unreleased/update-share.md @@ -0,0 +1,5 @@ +Enhancement: enable space members to update shares + +Enabled space members to update shares which they have not created themselves. + +https://github.com/cs3org/reva/pull/3192 diff --git a/pkg/share/manager/jsoncs3/jsoncs3.go b/pkg/share/manager/jsoncs3/jsoncs3.go index e56339c7db..e519636e35 100644 --- a/pkg/share/manager/jsoncs3/jsoncs3.go +++ b/pkg/share/manager/jsoncs3/jsoncs3.go @@ -28,12 +28,15 @@ import ( "github.com/pkg/errors" "google.golang.org/genproto/protobuf/field_mask" + gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" + "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/providercache" "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/receivedsharecache" @@ -41,6 +44,7 @@ import ( "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/shareid" "github.com/cs3org/reva/v2/pkg/share/manager/registry" "github.com/cs3org/reva/v2/pkg/storage/utils/metadata" // nolint:staticcheck // we need the legacy package to convert V1 to V2 messages + "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" ) @@ -120,6 +124,8 @@ type Manager struct { SpaceRoot *provider.ResourceId initialized bool + + gateway gatewayv1beta1.GatewayAPIClient } // NewDefault returns a new manager instance with default dependencies @@ -130,22 +136,28 @@ func NewDefault(m map[string]interface{}) (share.Manager, error) { return nil, err } - s, err := metadata.NewCS3Storage(c.GatewayAddr, c.ProviderAddr, c.ServiceUserID, c.ServiceUserIdp, c.MachineAuthAPIKey) + s, err := metadata.NewCS3Storage(c.ProviderAddr, c.ProviderAddr, c.ServiceUserID, c.ServiceUserIdp, c.MachineAuthAPIKey) + if err != nil { + return nil, err + } + + gc, err := pool.GetGatewayServiceClient(c.GatewayAddr) if err != nil { return nil, err } - return New(s) + return New(s, gc) } // New returns a new manager instance. -func New(s metadata.Storage) (*Manager, error) { +func New(s metadata.Storage, gc gatewayv1beta1.GatewayAPIClient) (*Manager, error) { return &Manager{ Cache: providercache.New(s), CreatedCache: sharecache.New(s, "users", "created.json"), UserReceivedStates: receivedsharecache.New(s), GroupReceivedCache: sharecache.New(s, "groups", "received.json"), storage: s, + gateway: gc, }, nil } @@ -355,6 +367,17 @@ func (m *Manager) GetShare(ctx context.Context, ref *collaboration.ShareReferenc if share.IsCreatedByUser(s, user) || share.IsGrantedToUser(s, user) { return s, nil } + + req := &provider.StatRequest{ + Ref: &provider.Reference{ResourceId: s.ResourceId}, + } + res, err := m.gateway.Stat(ctx, req) + if err == nil && + res.Status.Code == rpcv1beta1.Code_CODE_OK && + res.Info.PermissionSet.ListGrants { + return s, nil + } + // we return not found to not disclose information return nil, errtypes.NotFound(ref.String()) } @@ -425,7 +448,15 @@ func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareRefer user := ctxpkg.ContextMustGetUser(ctx) if !share.IsCreatedByUser(s, user) { - return nil, errtypes.NotFound(ref.String()) + req := &provider.StatRequest{ + Ref: &provider.Reference{ResourceId: s.ResourceId}, + } + res, err := m.gateway.Stat(ctx, req) + if err != nil || + res.Status.Code != rpcv1beta1.Code_CODE_OK || + !res.Info.PermissionSet.UpdateGrant { + return nil, errtypes.NotFound(ref.String()) + } } s.Permissions = p @@ -464,9 +495,8 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte defer m.Unlock() user := ctxpkg.ContextMustGetUser(ctx) - grouped := share.GroupFiltersByType(filters) - if len(grouped[collaboration.Filter_TYPE_RESOURCE_ID]) > 0 { + if len(share.FilterFiltersByType(filters, collaboration.Filter_TYPE_RESOURCE_ID)) > 0 { return m.listSharesByIDs(ctx, user, filters) } @@ -474,19 +504,18 @@ func (m *Manager) ListShares(ctx context.Context, filters []*collaboration.Filte } func (m *Manager) listSharesByIDs(ctx context.Context, user *userv1beta1.User, filters []*collaboration.Filter) ([]*collaboration.Share, error) { - var ss []*collaboration.Share - - grouped := share.GroupFiltersByType(filters) - providerSpaces := map[string]map[string]bool{} - for _, f := range grouped[collaboration.Filter_TYPE_RESOURCE_ID] { + providerSpaces := make(map[string]map[string]struct{}) + for _, f := range share.FilterFiltersByType(filters, collaboration.Filter_TYPE_RESOURCE_ID) { storageID := f.GetResourceId().GetStorageId() spaceID := f.GetResourceId().GetSpaceId() if providerSpaces[storageID] == nil { - providerSpaces[storageID] = map[string]bool{} + providerSpaces[storageID] = make(map[string]struct{}) } - providerSpaces[storageID][spaceID] = true + providerSpaces[storageID][spaceID] = struct{}{} } + statCache := make(map[string]struct{}) + var ss []*collaboration.Share for providerID, spaces := range providerSpaces { for spaceID := range spaces { err := m.Cache.Sync(ctx, providerID, spaceID) @@ -495,10 +524,29 @@ func (m *Manager) listSharesByIDs(ctx context.Context, user *userv1beta1.User, f } shares := m.Cache.ListSpace(providerID, spaceID) + for _, s := range shares.Shares { - if share.MatchesFilters(s, filters) { - ss = append(ss, s) + if !share.MatchesFilters(s, filters) { + continue } + + if !(share.IsCreatedByUser(s, user) || share.IsGrantedToUser(s, user)) { + key := storagespace.FormatResourceID(*s.ResourceId) + if _, hit := statCache[key]; !hit { + req := &provider.StatRequest{ + Ref: &provider.Reference{ResourceId: s.ResourceId}, + } + res, err := m.gateway.Stat(ctx, req) + if err != nil || + res.Status.Code != rpcv1beta1.Code_CODE_OK || + !res.Info.PermissionSet.ListGrants { + continue + } + statCache[key] = struct{}{} + } + } + + ss = append(ss, s) } } } diff --git a/pkg/share/manager/jsoncs3/jsoncs3_test.go b/pkg/share/manager/jsoncs3/jsoncs3_test.go index 2fcb29823d..689240fc44 100644 --- a/pkg/share/manager/jsoncs3/jsoncs3_test.go +++ b/pkg/share/manager/jsoncs3/jsoncs3_test.go @@ -34,11 +34,15 @@ import ( providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/conversions" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/rgrpc/status" sharespkg "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3" "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/sharecache" "github.com/cs3org/reva/v2/pkg/share/manager/jsoncs3/shareid" "github.com/cs3org/reva/v2/pkg/storage/utils/metadata" + "github.com/cs3org/reva/v2/pkg/utils" + "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" + "github.com/stretchr/testify/mock" "google.golang.org/protobuf/types/known/fieldmaskpb" . "github.com/onsi/ginkgo/v2" @@ -97,6 +101,7 @@ var _ = Describe("Jsoncs3", func() { }, } storage metadata.Storage + client *mocks.GatewayAPIClient tmpdir string m *jsoncs3.Manager @@ -115,6 +120,16 @@ var _ = Describe("Jsoncs3", func() { ExpectWithOffset(1, s).ToNot(BeNil()) return s } + + mockStat = func(ref *provider.Reference, info *provider.ResourceInfo) { + client.On("Stat", mock.Anything, mock.MatchedBy(func(req *provider.StatRequest) bool { + return utils.ResourceIDEqual(req.Ref.ResourceId, ref.ResourceId) && + (ref.Path == "" || req.Ref.Path == ref.Path) + })).Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: info, + }, nil) + } ) BeforeEach(func() { @@ -140,7 +155,8 @@ var _ = Describe("Jsoncs3", func() { storage, err = metadata.NewDiskStorage(tmpdir) Expect(err).ToNot(HaveOccurred()) - m, err = jsoncs3.New(storage) + client = &mocks.GatewayAPIClient{} + m, err = jsoncs3.New(storage, client) Expect(err).ToNot(HaveOccurred()) }) @@ -235,7 +251,7 @@ var _ = Describe("Jsoncs3", func() { }) Expect(s).ToNot(BeNil()) - m, err = jsoncs3.New(storage) // Reset in-memory cache + m, err = jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) s = shareBykey(&collaboration.ShareKey{ @@ -285,7 +301,14 @@ var _ = Describe("Jsoncs3", func() { }) Describe("ListShares", func() { + BeforeEach(func() { + mockStat( + &provider.Reference{ResourceId: sharedResource.Id}, + &providerv1beta1.ResourceInfo{PermissionSet: &providerv1beta1.ResourcePermissions{ListGrants: true}}, + ) + }) It("returns the share requested by id even though it's not owned or created by the manager", func() { + shares, err := m.ListShares(managerCtx, []*collaboration.Filter{ { Type: collaboration.Filter_TYPE_RESOURCE_ID, @@ -319,9 +342,16 @@ var _ = Describe("Jsoncs3", func() { }, }, } + }) Describe("GetShare", func() { + BeforeEach(func() { + mockStat( + &provider.Reference{ResourceId: sharedResource.Id}, + &providerv1beta1.ResourceInfo{PermissionSet: &providerv1beta1.ResourcePermissions{ListGrants: false}}, + ) + }) It("handles unknown ids", func() { s, err := m.GetShare(ctx, &collaboration.ShareReference{ Spec: &collaboration.ShareReference_Id{ @@ -415,7 +445,7 @@ var _ = Describe("Jsoncs3", func() { }) It("loads the cache when it doesn't have an entry", func() { - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) s, err := m.GetShare(ctx, shareRef) @@ -475,7 +505,7 @@ var _ = Describe("Jsoncs3", func() { }) Expect(err).ToNot(HaveOccurred()) - m, err = jsoncs3.New(storage) // Reset in-memory cache + m, err = jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) s, err := m.GetShare(ctx, &collaboration.ShareReference{ @@ -492,6 +522,12 @@ var _ = Describe("Jsoncs3", func() { }) Describe("UpdateShare", func() { + BeforeEach(func() { + mockStat( + &provider.Reference{ResourceId: sharedResource.Id}, + &providerv1beta1.ResourceInfo{PermissionSet: &providerv1beta1.ResourcePermissions{ListGrants: false}}, + ) + }) It("does not update shares of other users", func() { _, err := m.UpdateShare(otherCtx, &collaboration.ShareReference{ Spec: &collaboration.ShareReference_Id{ @@ -582,7 +618,7 @@ var _ = Describe("Jsoncs3", func() { Expect(us).ToNot(BeNil()) Expect(us.GetPermissions().GetPermissions().InitiateFileUpload).To(BeTrue()) - m, err = jsoncs3.New(storage) // Reset in-memory cache + m, err = jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) s = shareBykey(&collaboration.ShareKey{ @@ -713,7 +749,7 @@ var _ = Describe("Jsoncs3", func() { }) It("syncronizes the user received cache before listing", func() { - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) received, err := m.ListReceivedShares(granteeCtx, []*collaboration.Filter{}) @@ -781,7 +817,7 @@ var _ = Describe("Jsoncs3", func() { }) It("syncronizes the group received cache before listing", func() { - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) received, err := m.ListReceivedShares(granteeCtx, []*collaboration.Filter{}) @@ -825,7 +861,7 @@ var _ = Describe("Jsoncs3", func() { }) It("syncs the cache", func() { - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) rs, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{ @@ -859,7 +895,7 @@ var _ = Describe("Jsoncs3", func() { }) It("syncs the cache", func() { - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) rs, err := m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{ @@ -982,7 +1018,7 @@ var _ = Describe("Jsoncs3", func() { Expect(err).ToNot(HaveOccurred()) Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED)) - m, err := jsoncs3.New(storage) // Reset in-memory cache + m, err := jsoncs3.New(storage, nil) // Reset in-memory cache Expect(err).ToNot(HaveOccurred()) rs, err = m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{ diff --git a/pkg/share/share.go b/pkg/share/share.go index 2e7d53fd3d..f0f69f7689 100644 --- a/pkg/share/share.go +++ b/pkg/share/share.go @@ -204,3 +204,10 @@ func GroupFiltersByType(filters []*collaboration.Filter) map[collaboration.Filte } return grouped } + +// FilterFiltersByType returns a slice of filters by a given type. +// If no filter with the given type exists within the filters, then an +// empty slice is returned. +func FilterFiltersByType(f []*collaboration.Filter, t collaboration.Filter_Type) []*collaboration.Filter { + return GroupFiltersByType(f)[t] +}