diff --git a/internal/grpc/services/usershareprovider/usershareprovider.go b/internal/grpc/services/usershareprovider/usershareprovider.go index 21efd9bdfa..d18024d6f7 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider.go +++ b/internal/grpc/services/usershareprovider/usershareprovider.go @@ -318,6 +318,7 @@ 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) + user := ctxpkg.ContextMustGetUser(ctx) gatewayClient, err := s.gatewaySelector.Next() if err != nil { return nil, err @@ -365,6 +366,17 @@ func (s *service) UpdateShare(ctx context.Context, req *collaboration.UpdateShar Status: status.NewInternal(ctx, "failed to stat shared resource"), }, err } + // the requesting user needs to be either the Owner/Creator of the share or have the UpdateGrant permissions on the Resource + switch { + case utils.UserEqual(user.GetId(), currentShare.GetCreator()) || utils.UserEqual(user.GetId(), currentShare.GetOwner()): + fallthrough + case sRes.GetInfo().GetPermissionSet().UpdateGrant: + break + default: + return &collaboration.UpdateShareResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to remove grants on 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 diff --git a/internal/grpc/services/usershareprovider/usershareprovider_test.go b/internal/grpc/services/usershareprovider/usershareprovider_test.go index b93ddad02b..7e9b4347a1 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider_test.go +++ b/internal/grpc/services/usershareprovider/usershareprovider_test.go @@ -56,6 +56,7 @@ var _ = Describe("user share provider service", func() { checkPermissionResponse *permissions.CheckPermissionResponse statResourceResponse *providerpb.StatResponse cs3permissionsNoAddGrant *providerpb.ResourcePermissions + getShareResponse *collaborationpb.Share ) cs3permissionsNoAddGrant = conversions.RoleFromName("manager", true).CS3ResourcePermissions() cs3permissionsNoAddGrant.AddGrant = false @@ -90,8 +91,14 @@ var _ = Describe("user share provider service", func() { }, } gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResourceResponse, nil) + alice := &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "alice", + }, + Username: "alice", + } - getShareResponse := &collaborationpb.Share{ + getShareResponse = &collaborationpb.Share{ Id: &collaborationpb.ShareId{ OpaqueId: "shareid", }, @@ -100,6 +107,8 @@ var _ = Describe("user share provider service", func() { SpaceId: "spaceid", OpaqueId: "opaqueid", }, + Owner: alice.Id, + Creator: alice.Id, } manager.On("GetShare", mock.Anything, mock.Anything).Return(getShareResponse, nil) @@ -108,12 +117,7 @@ var _ = Describe("user share provider service", func() { provider = rgrpcService.(collaborationpb.CollaborationAPIServer) Expect(provider).ToNot(BeNil()) - ctx = ctxpkg.ContextSetUser(context.Background(), &userpb.User{ - Id: &userpb.UserId{ - OpaqueId: "alice", - }, - Username: "alice", - }) + ctx = ctxpkg.ContextSetUser(context.Background(), alice) }) Describe("CreateShare", func() { @@ -206,7 +210,6 @@ var _ = Describe("user share provider service", func() { It("fails when the user tries to share with elevated permissions", func() { // user has only read access statResourceResponse.Info.PermissionSet = &providerpb.ResourcePermissions{ - UpdateGrant: true, InitiateFileDownload: true, Stat: true, } @@ -239,13 +242,87 @@ var _ = Describe("user share provider service", func() { manager.AssertNumberOfCalls(GinkgoT(), "UpdateShare", 0) }) - It("succeeds when the user has sufficient permissions", func() { + It("succeeds when the user is not the owner/creator and does not have the UpdateGrant permissions", func() { + // user has only read access + statResourceResponse.Info.PermissionSet = &providerpb.ResourcePermissions{ + InitiateFileDownload: true, + Stat: true, + } + bobId := &userpb.UserId{OpaqueId: "bob"} + getShareResponse.Owner = bobId + getShareResponse.Creator = bobId + + // user tries to update a share to give write permissions + updateShareResponse, err := provider.UpdateShare(ctx, &collaborationpb.UpdateShareRequest{ + Ref: &collaborationpb.ShareReference{ + Spec: &collaborationpb.ShareReference_Id{ + Id: &collaborationpb.ShareId{ + OpaqueId: "shareid", + }, + }, + }, + Share: &collaborationpb.Share{ + Permissions: &collaborationpb.SharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + InitiateFileDownload: true, + }, + }, + }, + UpdateMask: &fieldmaskpb.FieldMask{ + Paths: []string{"permissions"}, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(updateShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_PERMISSION_DENIED)) + + manager.AssertNumberOfCalls(GinkgoT(), "UpdateShare", 0) + }) + It("succeeds when the user is the owner/creator", func() { + // user has only read access + statResourceResponse.Info.PermissionSet = &providerpb.ResourcePermissions{ + InitiateFileDownload: true, + Stat: true, + } + + // user tries to update a share to give write permissions + updateShareResponse, err := provider.UpdateShare(ctx, &collaborationpb.UpdateShareRequest{ + Ref: &collaborationpb.ShareReference{ + Spec: &collaborationpb.ShareReference_Id{ + Id: &collaborationpb.ShareId{ + OpaqueId: "shareid", + }, + }, + }, + Share: &collaborationpb.Share{ + Permissions: &collaborationpb.SharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + InitiateFileDownload: true, + }, + }, + }, + UpdateMask: &fieldmaskpb.FieldMask{ + Paths: []string{"permissions"}, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(updateShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_OK)) + + manager.AssertNumberOfCalls(GinkgoT(), "UpdateShare", 1) + }) + It("succeeds when the user is not the owner/creator but has the UpdateGrant permissions", func() { // user has only read access statResourceResponse.Info.PermissionSet = &providerpb.ResourcePermissions{ UpdateGrant: true, InitiateFileDownload: true, Stat: true, } + bobId := &userpb.UserId{OpaqueId: "bob"} + getShareResponse.Owner = bobId + getShareResponse.Creator = bobId // user tries to update a share to give write permissions updateShareResponse, err := provider.UpdateShare(ctx, &collaborationpb.UpdateShareRequest{