From a56eee1b18e3bd9fd4d05421ee7bae953125297b Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 10 Apr 2024 16:26:31 +0200 Subject: [PATCH 1/4] tests: make publicshareprovider testable --- .mockery.yaml | 3 + .../fix-public-share-update.md | 1 + .../publicshareprovider/expose_test.go | 6 + .../publicshareprovider.go | 21 +- .../publicshareprovider_suite_test.go | 13 + .../publicshareprovider_test.go | 924 ++++++++++++++++++ pkg/publicshare/mocks/Manager.go | 411 ++++++++ 7 files changed, 1370 insertions(+), 9 deletions(-) create mode 100644 internal/grpc/services/publicshareprovider/expose_test.go create mode 100644 internal/grpc/services/publicshareprovider/publicshareprovider_suite_test.go create mode 100644 internal/grpc/services/publicshareprovider/publicshareprovider_test.go create mode 100644 pkg/publicshare/mocks/Manager.go diff --git a/.mockery.yaml b/.mockery.yaml index 839791b141..53c69f4258 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -7,6 +7,9 @@ packages: github.com/cs3org/reva/v2/pkg/publicshare/manager/owncloudsql: interfaces: UserConverter: + github.com/cs3org/reva/v2/pkg/publicshare: + interfaces: + Manager: github.com/cs3org/reva/v2/pkg/share: interfaces: Manager: diff --git a/changelog/2.20.0_2024-06-19/fix-public-share-update.md b/changelog/2.20.0_2024-06-19/fix-public-share-update.md index ce0fb4f490..acf5a1283e 100644 --- a/changelog/2.20.0_2024-06-19/fix-public-share-update.md +++ b/changelog/2.20.0_2024-06-19/fix-public-share-update.md @@ -2,4 +2,5 @@ Bugfix: Fix public share update We fixed the permission check for updating public shares. When updating the permissions of a public share while not providing a password, the check must be against the new permissions to take into account that users can opt out only for view permissions. +https://github.com/cs3org/reva/pull/4633 https://github.com/cs3org/reva/pull/4622 \ No newline at end of file diff --git a/internal/grpc/services/publicshareprovider/expose_test.go b/internal/grpc/services/publicshareprovider/expose_test.go new file mode 100644 index 0000000000..3931a003f0 --- /dev/null +++ b/internal/grpc/services/publicshareprovider/expose_test.go @@ -0,0 +1,6 @@ +package publicshareprovider + +var ( + ParseConfig = parseConfig + ParsePasswordPolicy = parsePasswordPolicy +) diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index 3202b8c755..4e58661916 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -53,7 +53,7 @@ import ( const getUserCtxErrMsg = "error getting user from context" func init() { - rgrpc.Register("publicshareprovider", New) + rgrpc.Register("publicshareprovider", NewDefault) } type config struct { @@ -127,9 +127,8 @@ func parsePasswordPolicy(m map[string]interface{}) (*passwordPolicy, error) { return p, nil } -// New creates a new user share provider svc -func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { - +// New creates a new public share provider svc initialized from defaults +func NewDefault(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { c, err := parseConfig(m) if err != nil { return nil, err @@ -146,6 +145,15 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { return nil, err } + gatewaySelector, err := pool.GatewaySelector(sharedconf.GetGatewaySVC(c.GatewayAddr)) + if err != nil { + return nil, err + } + return New(gatewaySelector, sm, c, p) +} + +// New creates a new user share provider svc +func New(gatewaySelector pool.Selectable[gateway.GatewayAPIClient], sm publicshare.Manager, c *config, p *passwordPolicy) (rgrpc.Service, error) { allowedPathsForShares := make([]*regexp.Regexp, 0, len(c.AllowedPathsForShares)) for _, s := range c.AllowedPathsForShares { regex, err := regexp.Compile(s) @@ -155,11 +163,6 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { allowedPathsForShares = append(allowedPathsForShares, regex) } - gatewaySelector, err := pool.GatewaySelector(sharedconf.GetGatewaySVC(c.GatewayAddr)) - if err != nil { - return nil, err - } - service := &service{ conf: c, sm: sm, diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider_suite_test.go b/internal/grpc/services/publicshareprovider/publicshareprovider_suite_test.go new file mode 100644 index 0000000000..a3b911ce8f --- /dev/null +++ b/internal/grpc/services/publicshareprovider/publicshareprovider_suite_test.go @@ -0,0 +1,13 @@ +package publicshareprovider_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestPublicShareProvider(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "PublicShareProvider Suite") +} diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider_test.go b/internal/grpc/services/publicshareprovider/publicshareprovider_test.go new file mode 100644 index 0000000000..3d63353147 --- /dev/null +++ b/internal/grpc/services/publicshareprovider/publicshareprovider_test.go @@ -0,0 +1,924 @@ +package publicshareprovider_test + +import ( + "context" + "time" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" + providerpb "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/v2/internal/grpc/services/publicshareprovider" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/publicshare" + "github.com/cs3org/reva/v2/pkg/publicshare/manager/registry" + "github.com/cs3org/reva/v2/pkg/publicshare/mocks" + "github.com/cs3org/reva/v2/pkg/rgrpc/status" + "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/utils" + cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/pkg/errors" + "github.com/stretchr/testify/mock" + "google.golang.org/grpc" +) + +func createPublicShareProvider(revaConfig map[string]interface{}, gs pool.Selectable[gateway.GatewayAPIClient], sm publicshare.Manager) (link.LinkAPIServer, error) { + config, err := publicshareprovider.ParseConfig(revaConfig) + if err != nil { + return nil, err + } + passwordPolicy, err := publicshareprovider.ParsePasswordPolicy(config.PasswordPolicy) + if err != nil { + return nil, err + } + publicshareproviderService, err := publicshareprovider.New(gs, sm, config, passwordPolicy) + if err != nil { + return nil, err + } + return publicshareproviderService.(link.LinkAPIServer), nil +} + +var _ = Describe("PublicShareProvider", func() { + // declare in container nodes + var ( + ctx context.Context + checkPermissionResponse *permissions.CheckPermissionResponse + statResourceResponse *providerpb.StatResponse + provider link.LinkAPIServer + manager *mocks.Manager + gatewayClient *cs3mocks.GatewayAPIClient + gatewaySelector pool.Selectable[gateway.GatewayAPIClient] + resourcePermissions *providerpb.ResourcePermissions + linkPermissions *providerpb.ResourcePermissions + createdLink *link.PublicShare + revaConfig map[string]interface{} + user *userpb.User + ) + + BeforeEach(func() { + // initialize in setup nodes + manager = mocks.NewManager(GinkgoT()) + + registry.Register("mockManager", func(m map[string]interface{}) (publicshare.Manager, error) { + return manager, nil + }) + + pool.RemoveSelector("GatewaySelector" + "any") + gatewayClient = cs3mocks.NewGatewayAPIClient(GinkgoT()) + gatewaySelector = pool.GetSelector[gateway.GatewayAPIClient]( + "GatewaySelector", + "any", + func(cc grpc.ClientConnInterface) gateway.GatewayAPIClient { + return gatewayClient + }, + ) + + checkPermissionResponse = &permissions.CheckPermissionResponse{ + Status: status.NewOK(ctx), + } + + user = &userpb.User{ + Id: &userpb.UserId{ + OpaqueId: "alice", + }, + Username: "alice", + } + ctx = ctxpkg.ContextSetUser(context.Background(), user) + + resourcePermissions = &providerpb.ResourcePermissions{ + // all permissions + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + ListContainer: true, + ListFileVersions: true, + ListGrants: true, + ListRecycle: true, + Move: true, + PurgeRecycle: true, + RemoveGrant: true, + RestoreFileVersion: true, + RestoreRecycleItem: true, + Stat: true, + UpdateGrant: true, + DenyGrant: true, + } + + statResourceResponse = &providerpb.StatResponse{ + Status: status.NewOK(ctx), + Info: &providerpb.ResourceInfo{ + Type: providerpb.ResourceType_RESOURCE_TYPE_FILE, + Path: "./file.txt", + PermissionSet: resourcePermissions, + }, + } + + linkPermissions = &providerpb.ResourcePermissions{ + Stat: true, + CreateContainer: true, + Delete: true, + GetPath: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + } + + createdLink = &link.PublicShare{ + PasswordProtected: true, + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Creator: user.Id, + } + + revaConfig = map[string]interface{}{ + "driver": "mockManager", + "drivers": map[string]map[string]interface{}{ + "jsoncs3": { + "provider_addr": "com.owncloud.api.storage-system", + "service_user_idp": "internal", + "enable_expired_shares_cleanup": true, + "gateway_addr": "https://localhost:9200", + }, + }, + "gateway_addr": "https://localhost:9200", + "allowed_paths_for_shares": []string{"/NewFolder"}, + "writeable_share_must_have_password": false, + "public_share_must_have_password": true, + "password_policy": map[string]interface{}{ + "min_digits": 1, + "min_characters": 8, + "min_lowercase_characters": 1, + "min_uppercase_characters": 1, + "min_special_characters": 1, + "banned_passwords_list": map[string]struct{}{ + "SecretPassword1!": {}, + }, + }, + } + var err error + provider, err = createPublicShareProvider(revaConfig, gatewaySelector, manager) + Expect(err).ToNot(HaveOccurred()) + Expect(provider).ToNot(BeNil()) + }) + Describe("Creating a PublicShare", func() { + BeforeEach(func() { + gatewayClient. + EXPECT(). + CheckPermission( + mock.Anything, + mock.Anything, + ). + Return(checkPermissionResponse, nil) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(statResourceResponse, nil) + }) + It("creates a public share with password", func() { + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Password: "SecretPassw0rd!", + }, + Description: "test", + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("has no user permission to create public share", func() { + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Unset() + checkPermissionResponse = &permissions.CheckPermissionResponse{ + Status: status.NewPermissionDenied(ctx, nil, "permission denied"), + } + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(checkPermissionResponse, nil) + + req := &link.CreatePublicShareRequest{} + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + Expect(res.GetStatus().GetMessage()).To(Equal("no permission to create public links")) + }) + It("has permission to create internal link", func() { + // internal links are created with empty permissions + linkPermissions := &providerpb.ResourcePermissions{} + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + // internal link creation should not check user permissions + gatewayClient.EXPECT().CheckPermission(mock.Anything, mock.Anything).Unset() + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + Description: "test", + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("has no share permission on the resource to create internal link", func() { + // internal links are created with empty permissions + linkPermissions := &providerpb.ResourcePermissions{} + + // internal link creation should not check user permissions + gatewayClient.EXPECT().CheckPermission(mock.Anything, mock.Anything).Unset() + + // downgrade user permissions on resource to have no share permission + resourcePermissions.AddGrant = false + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + Description: "test", + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("no share permission")) + }) + It("fails to check create public share user permission", func() { + gatewayClient.EXPECT().CheckPermission(mock.Anything, mock.Anything).Unset() + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.CreatePublicShareRequest{} + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("failed check user permission to write public link")) + }) + It("has no share permission on the resource", func() { + req := &link.CreatePublicShareRequest{} + + // downgrade user permissions to have no share permission + resourcePermissions.AddGrant = false + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("no share permission")) + }) + It("tries to share with higher permissions than granted on the resource", func() { + // set resource permissions lower than the requested share permissions + resourcePermissions.Delete = false + + req := &link.CreatePublicShareRequest{ + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("insufficient permissions to create that kind of share")) + }) + It("tries to share inside a path which is not allowed", func() { + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_FAILED_PRECONDITION)) + Expect(res.GetStatus().GetMessage()).To(Equal("share creation is not allowed for the specified path")) + }) + It("tries to share personal space root", func() { + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + Id: &providerpb.ResourceId{ + StorageId: "storage-id", + OpaqueId: "admin-id", + SpaceId: "admin-id", + }, + Space: &providerpb.StorageSpace{SpaceType: "personal"}, + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("cannot create link on personal space root")) + }) + It("tries to share a project space root", func() { + manager.EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + Id: &providerpb.ResourceId{ + StorageId: "storage-id", + OpaqueId: "project-id", + SpaceId: "project-id", + }, + Space: &providerpb.StorageSpace{SpaceType: "project"}, + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Password: "SecretPassw0rd!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("creates a new quicklink", func() { + createdLink.Quicklink = true + manager.EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + // no existing quicklinks + var links = []*link.PublicShare{} + manager.EXPECT().ListPublicShares(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(links, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + ArbitraryMetadata: &providerpb.ArbitraryMetadata{Metadata: map[string]string{"quicklink": "true"}}, + Path: "./NewFolder/file.txt", + Id: &providerpb.ResourceId{ + StorageId: "storage-id", + OpaqueId: "project-id", + SpaceId: "project-id", + }, + Space: &providerpb.StorageSpace{SpaceType: "project"}, + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Password: "SecretPassw0rd!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("creates a quicklink which already exists", func() { + // create a quicklink with viewer permissions + // there is already a quicklink present for that resource + // instead of creating a new quicklink, we return the existing quicklink without updating it + existingLink := &link.PublicShare{ + PasswordProtected: true, + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Quicklink: true, + Description: "Quicklink", + } + var links = []*link.PublicShare{} + links = append(links, existingLink) + + // confirm that list public shares is called with the correct filters + manager. + EXPECT(). + ListPublicShares( + mock.Anything, + mock.Anything, + mock.MatchedBy(func(filters []*link.ListPublicSharesRequest_Filter) bool { + return filters[0].Type == link.ListPublicSharesRequest_Filter_TYPE_RESOURCE_ID && + filters[0].GetResourceId().GetOpaqueId() == "project-id" && + filters[0].GetResourceId().GetStorageId() == "storage-id" && + filters[0].GetResourceId().GetSpaceId() == "project-id" + }), + mock.Anything). + Return(links, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + // this should create a quicklink + ArbitraryMetadata: &providerpb.ArbitraryMetadata{Metadata: map[string]string{"quicklink": "true"}}, + Path: "./NewFolder/file.txt", + Id: &providerpb.ResourceId{ + StorageId: "storage-id", + OpaqueId: "project-id", + SpaceId: "project-id", + }, + Space: &providerpb.StorageSpace{SpaceType: "project"}, + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + InitiateFileDownload: true, + ListContainer: true, + }, + }, + Password: "SecretPassw0rd!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(existingLink)) + }) + It("create public share with expiration date in the past", func() { + yesterday := time.Now().AddDate(0, 0, -1) + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Expiration: utils.TimeToTS(yesterday), + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("expiration date is in the past")) + }) + It("create public share with valid expiration", func() { + tomorrow := time.Now().AddDate(0, 0, 1) + tsTomorrow := utils.TimeToTS(tomorrow) + + createdLink.Expiration = tsTomorrow + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Expiration: tsTomorrow, + Password: "SecretPassw0rd!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("creates a public share without a password", func() { + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("password protection is enforced")) + }) + It("creates a viewable public share without a password", func() { + // set password enforcement only on writeable shares + revaConfig["public_share_must_have_password"] = false + revaConfig["writeable_share_must_have_password"] = true + + provider, err := createPublicShareProvider(revaConfig, gatewaySelector, manager) + Expect(err).ToNot(HaveOccurred()) + Expect(provider).ToNot(BeNil()) + + linkPermissions = &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + } + createdLink.PasswordProtected = false + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("creates an editable public share without a password", func() { + // set password enforcement to false on all public shares + revaConfig["public_share_must_have_password"] = false + revaConfig["writeable_share_must_have_password"] = false + + provider, err := createPublicShareProvider(revaConfig, gatewaySelector, manager) + Expect(err).ToNot(HaveOccurred()) + Expect(provider).ToNot(BeNil()) + + linkPermissions = &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + CreateContainer: true, + Delete: true, + Move: true, + } + createdLink.PasswordProtected = false + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) + }) + It("applies the password policy even if no enforcement is configured", func() { + // set password enforcement to false on all public shares + revaConfig["public_share_must_have_password"] = false + revaConfig["writeable_share_must_have_password"] = false + + provider, err := createPublicShareProvider(revaConfig, gatewaySelector, manager) + Expect(err).ToNot(HaveOccurred()) + Expect(provider).ToNot(BeNil()) + + linkPermissions = &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + CreateContainer: true, + Delete: true, + Move: true, + } + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Password: "SecretPassword1!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("unfortunately, your password is commonly used. please pick a harder-to-guess password for your safety")) + }) + It("returns internal server error when share manager not functional", func() { + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.CreatePublicShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + Owner: &userpb.UserId{ + OpaqueId: "alice", + }, + Path: "./NewFolder/file.txt", + }, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Password: "SecretPassw0rd123!", + }, + } + + res, err := provider.CreatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("error persisting public share:transport error")) + }) + }) + Describe("Removing a PublicShare", func() { + BeforeEach(func() { + createdLink.Id = &link.PublicShareId{ + OpaqueId: "share-id", + } + }) + It("cannot load existing share", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("error loading public share")) + }) + It("can remove an existing share as a creator", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once(). + Return( + createdLink, nil) + manager. + EXPECT(). + RevokePublicShare( + mock.Anything, + mock.MatchedBy(func(callingUser *userpb.User) bool { + return callingUser == user + }), + mock.MatchedBy(func(linkRef *link.PublicShareReference) bool { + return linkRef.GetId().GetOpaqueId() == "share-id" + })). + Once(). + Return(nil) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + }) + It("can remove an existing share as a space manager", func() { + // link is neither owned nor created by the acting user + createdLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(createdLink, nil) + + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything).Return(statResourceResponse, nil) + + manager. + EXPECT(). + RevokePublicShare( + mock.Anything, + mock.MatchedBy( + func(callingUser *userpb.User) bool { + return callingUser == user + }), + mock.MatchedBy( + func(linkRef *link.PublicShareReference) bool { + return linkRef.GetId().GetOpaqueId() == "share-id" + })). + Once().Return(nil) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + }) + Context("when the user is not the owner or creator", func() { + BeforeEach(func() { + // link is neither owned nor created by the acting user + createdLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + + // stat the shared resource to get the users resource permissions + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(statResourceResponse, nil) + }) + It("cannot remove an existing share as a space editor", func() { + // downgrade user permissions to editor + resourcePermissions.AddGrant = false + resourcePermissions.UpdateGrant = false + resourcePermissions.RemoveGrant = false + + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(createdLink, nil) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + Expect(res.GetStatus().GetMessage()).To(Equal("no permission to delete public share")) + }) + It("triggers an internal server error when the share manager is not available", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(createdLink, nil) + + // Share manager is not functional + By("experiencing a share delete error in the persistence layer") + manager.EXPECT().RevokePublicShare(mock.Anything, mock.Anything, mock.Anything). + Once().Return(errors.New("delete error")) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("error deleting public share")) + }) + It("triggers an internal server error when the storage provider is not functional", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(createdLink, nil) + + // Storage Provider is not functional + By("experiencing a transport error when trying to stat the shared resource") + gatewayClient.EXPECT(). + Stat(mock.Anything, mock.Anything).Unset() + gatewayClient.EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + res, err := provider.RemovePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("failed to stat shared resource")) + }) + }) + }) +}) diff --git a/pkg/publicshare/mocks/Manager.go b/pkg/publicshare/mocks/Manager.go new file mode 100644 index 0000000000..403a576135 --- /dev/null +++ b/pkg/publicshare/mocks/Manager.go @@ -0,0 +1,411 @@ +// Copyright 2018-2022 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +// Code generated by mockery v2.40.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + + linkv1beta1 "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" + mock "github.com/stretchr/testify/mock" + + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + + userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" +) + +// Manager is an autogenerated mock type for the Manager type +type Manager struct { + mock.Mock +} + +type Manager_Expecter struct { + mock *mock.Mock +} + +func (_m *Manager) EXPECT() *Manager_Expecter { + return &Manager_Expecter{mock: &_m.Mock} +} + +// CreatePublicShare provides a mock function with given fields: ctx, u, md, g +func (_m *Manager) CreatePublicShare(ctx context.Context, u *userv1beta1.User, md *providerv1beta1.ResourceInfo, g *linkv1beta1.Grant) (*linkv1beta1.PublicShare, error) { + ret := _m.Called(ctx, u, md, g) + + if len(ret) == 0 { + panic("no return value specified for CreatePublicShare") + } + + var r0 *linkv1beta1.PublicShare + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *providerv1beta1.ResourceInfo, *linkv1beta1.Grant) (*linkv1beta1.PublicShare, error)); ok { + return rf(ctx, u, md, g) + } + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *providerv1beta1.ResourceInfo, *linkv1beta1.Grant) *linkv1beta1.PublicShare); ok { + r0 = rf(ctx, u, md, g) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*linkv1beta1.PublicShare) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *userv1beta1.User, *providerv1beta1.ResourceInfo, *linkv1beta1.Grant) error); ok { + r1 = rf(ctx, u, md, g) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Manager_CreatePublicShare_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreatePublicShare' +type Manager_CreatePublicShare_Call struct { + *mock.Call +} + +// CreatePublicShare is a helper method to define mock.On call +// - ctx context.Context +// - u *userv1beta1.User +// - md *providerv1beta1.ResourceInfo +// - g *linkv1beta1.Grant +func (_e *Manager_Expecter) CreatePublicShare(ctx interface{}, u interface{}, md interface{}, g interface{}) *Manager_CreatePublicShare_Call { + return &Manager_CreatePublicShare_Call{Call: _e.mock.On("CreatePublicShare", ctx, u, md, g)} +} + +func (_c *Manager_CreatePublicShare_Call) Run(run func(ctx context.Context, u *userv1beta1.User, md *providerv1beta1.ResourceInfo, g *linkv1beta1.Grant)) *Manager_CreatePublicShare_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*userv1beta1.User), args[2].(*providerv1beta1.ResourceInfo), args[3].(*linkv1beta1.Grant)) + }) + return _c +} + +func (_c *Manager_CreatePublicShare_Call) Return(_a0 *linkv1beta1.PublicShare, _a1 error) *Manager_CreatePublicShare_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Manager_CreatePublicShare_Call) RunAndReturn(run func(context.Context, *userv1beta1.User, *providerv1beta1.ResourceInfo, *linkv1beta1.Grant) (*linkv1beta1.PublicShare, error)) *Manager_CreatePublicShare_Call { + _c.Call.Return(run) + return _c +} + +// GetPublicShare provides a mock function with given fields: ctx, u, ref, sign +func (_m *Manager) GetPublicShare(ctx context.Context, u *userv1beta1.User, ref *linkv1beta1.PublicShareReference, sign bool) (*linkv1beta1.PublicShare, error) { + ret := _m.Called(ctx, u, ref, sign) + + if len(ret) == 0 { + panic("no return value specified for GetPublicShare") + } + + var r0 *linkv1beta1.PublicShare + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference, bool) (*linkv1beta1.PublicShare, error)); ok { + return rf(ctx, u, ref, sign) + } + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference, bool) *linkv1beta1.PublicShare); ok { + r0 = rf(ctx, u, ref, sign) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*linkv1beta1.PublicShare) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference, bool) error); ok { + r1 = rf(ctx, u, ref, sign) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Manager_GetPublicShare_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetPublicShare' +type Manager_GetPublicShare_Call struct { + *mock.Call +} + +// GetPublicShare is a helper method to define mock.On call +// - ctx context.Context +// - u *userv1beta1.User +// - ref *linkv1beta1.PublicShareReference +// - sign bool +func (_e *Manager_Expecter) GetPublicShare(ctx interface{}, u interface{}, ref interface{}, sign interface{}) *Manager_GetPublicShare_Call { + return &Manager_GetPublicShare_Call{Call: _e.mock.On("GetPublicShare", ctx, u, ref, sign)} +} + +func (_c *Manager_GetPublicShare_Call) Run(run func(ctx context.Context, u *userv1beta1.User, ref *linkv1beta1.PublicShareReference, sign bool)) *Manager_GetPublicShare_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*userv1beta1.User), args[2].(*linkv1beta1.PublicShareReference), args[3].(bool)) + }) + return _c +} + +func (_c *Manager_GetPublicShare_Call) Return(_a0 *linkv1beta1.PublicShare, _a1 error) *Manager_GetPublicShare_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Manager_GetPublicShare_Call) RunAndReturn(run func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference, bool) (*linkv1beta1.PublicShare, error)) *Manager_GetPublicShare_Call { + _c.Call.Return(run) + return _c +} + +// GetPublicShareByToken provides a mock function with given fields: ctx, token, auth, sign +func (_m *Manager) GetPublicShareByToken(ctx context.Context, token string, auth *linkv1beta1.PublicShareAuthentication, sign bool) (*linkv1beta1.PublicShare, error) { + ret := _m.Called(ctx, token, auth, sign) + + if len(ret) == 0 { + panic("no return value specified for GetPublicShareByToken") + } + + var r0 *linkv1beta1.PublicShare + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, *linkv1beta1.PublicShareAuthentication, bool) (*linkv1beta1.PublicShare, error)); ok { + return rf(ctx, token, auth, sign) + } + if rf, ok := ret.Get(0).(func(context.Context, string, *linkv1beta1.PublicShareAuthentication, bool) *linkv1beta1.PublicShare); ok { + r0 = rf(ctx, token, auth, sign) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*linkv1beta1.PublicShare) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string, *linkv1beta1.PublicShareAuthentication, bool) error); ok { + r1 = rf(ctx, token, auth, sign) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Manager_GetPublicShareByToken_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetPublicShareByToken' +type Manager_GetPublicShareByToken_Call struct { + *mock.Call +} + +// GetPublicShareByToken is a helper method to define mock.On call +// - ctx context.Context +// - token string +// - auth *linkv1beta1.PublicShareAuthentication +// - sign bool +func (_e *Manager_Expecter) GetPublicShareByToken(ctx interface{}, token interface{}, auth interface{}, sign interface{}) *Manager_GetPublicShareByToken_Call { + return &Manager_GetPublicShareByToken_Call{Call: _e.mock.On("GetPublicShareByToken", ctx, token, auth, sign)} +} + +func (_c *Manager_GetPublicShareByToken_Call) Run(run func(ctx context.Context, token string, auth *linkv1beta1.PublicShareAuthentication, sign bool)) *Manager_GetPublicShareByToken_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(*linkv1beta1.PublicShareAuthentication), args[3].(bool)) + }) + return _c +} + +func (_c *Manager_GetPublicShareByToken_Call) Return(_a0 *linkv1beta1.PublicShare, _a1 error) *Manager_GetPublicShareByToken_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Manager_GetPublicShareByToken_Call) RunAndReturn(run func(context.Context, string, *linkv1beta1.PublicShareAuthentication, bool) (*linkv1beta1.PublicShare, error)) *Manager_GetPublicShareByToken_Call { + _c.Call.Return(run) + return _c +} + +// ListPublicShares provides a mock function with given fields: ctx, u, filters, sign +func (_m *Manager) ListPublicShares(ctx context.Context, u *userv1beta1.User, filters []*linkv1beta1.ListPublicSharesRequest_Filter, sign bool) ([]*linkv1beta1.PublicShare, error) { + ret := _m.Called(ctx, u, filters, sign) + + if len(ret) == 0 { + panic("no return value specified for ListPublicShares") + } + + var r0 []*linkv1beta1.PublicShare + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, []*linkv1beta1.ListPublicSharesRequest_Filter, bool) ([]*linkv1beta1.PublicShare, error)); ok { + return rf(ctx, u, filters, sign) + } + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, []*linkv1beta1.ListPublicSharesRequest_Filter, bool) []*linkv1beta1.PublicShare); ok { + r0 = rf(ctx, u, filters, sign) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*linkv1beta1.PublicShare) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *userv1beta1.User, []*linkv1beta1.ListPublicSharesRequest_Filter, bool) error); ok { + r1 = rf(ctx, u, filters, sign) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Manager_ListPublicShares_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ListPublicShares' +type Manager_ListPublicShares_Call struct { + *mock.Call +} + +// ListPublicShares is a helper method to define mock.On call +// - ctx context.Context +// - u *userv1beta1.User +// - filters []*linkv1beta1.ListPublicSharesRequest_Filter +// - sign bool +func (_e *Manager_Expecter) ListPublicShares(ctx interface{}, u interface{}, filters interface{}, sign interface{}) *Manager_ListPublicShares_Call { + return &Manager_ListPublicShares_Call{Call: _e.mock.On("ListPublicShares", ctx, u, filters, sign)} +} + +func (_c *Manager_ListPublicShares_Call) Run(run func(ctx context.Context, u *userv1beta1.User, filters []*linkv1beta1.ListPublicSharesRequest_Filter, sign bool)) *Manager_ListPublicShares_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*userv1beta1.User), args[2].([]*linkv1beta1.ListPublicSharesRequest_Filter), args[3].(bool)) + }) + return _c +} + +func (_c *Manager_ListPublicShares_Call) Return(_a0 []*linkv1beta1.PublicShare, _a1 error) *Manager_ListPublicShares_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Manager_ListPublicShares_Call) RunAndReturn(run func(context.Context, *userv1beta1.User, []*linkv1beta1.ListPublicSharesRequest_Filter, bool) ([]*linkv1beta1.PublicShare, error)) *Manager_ListPublicShares_Call { + _c.Call.Return(run) + return _c +} + +// RevokePublicShare provides a mock function with given fields: ctx, u, ref +func (_m *Manager) RevokePublicShare(ctx context.Context, u *userv1beta1.User, ref *linkv1beta1.PublicShareReference) error { + ret := _m.Called(ctx, u, ref) + + if len(ret) == 0 { + panic("no return value specified for RevokePublicShare") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference) error); ok { + r0 = rf(ctx, u, ref) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Manager_RevokePublicShare_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RevokePublicShare' +type Manager_RevokePublicShare_Call struct { + *mock.Call +} + +// RevokePublicShare is a helper method to define mock.On call +// - ctx context.Context +// - u *userv1beta1.User +// - ref *linkv1beta1.PublicShareReference +func (_e *Manager_Expecter) RevokePublicShare(ctx interface{}, u interface{}, ref interface{}) *Manager_RevokePublicShare_Call { + return &Manager_RevokePublicShare_Call{Call: _e.mock.On("RevokePublicShare", ctx, u, ref)} +} + +func (_c *Manager_RevokePublicShare_Call) Run(run func(ctx context.Context, u *userv1beta1.User, ref *linkv1beta1.PublicShareReference)) *Manager_RevokePublicShare_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*userv1beta1.User), args[2].(*linkv1beta1.PublicShareReference)) + }) + return _c +} + +func (_c *Manager_RevokePublicShare_Call) Return(_a0 error) *Manager_RevokePublicShare_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *Manager_RevokePublicShare_Call) RunAndReturn(run func(context.Context, *userv1beta1.User, *linkv1beta1.PublicShareReference) error) *Manager_RevokePublicShare_Call { + _c.Call.Return(run) + return _c +} + +// UpdatePublicShare provides a mock function with given fields: ctx, u, req +func (_m *Manager) UpdatePublicShare(ctx context.Context, u *userv1beta1.User, req *linkv1beta1.UpdatePublicShareRequest) (*linkv1beta1.PublicShare, error) { + ret := _m.Called(ctx, u, req) + + if len(ret) == 0 { + panic("no return value specified for UpdatePublicShare") + } + + var r0 *linkv1beta1.PublicShare + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *linkv1beta1.UpdatePublicShareRequest) (*linkv1beta1.PublicShare, error)); ok { + return rf(ctx, u, req) + } + if rf, ok := ret.Get(0).(func(context.Context, *userv1beta1.User, *linkv1beta1.UpdatePublicShareRequest) *linkv1beta1.PublicShare); ok { + r0 = rf(ctx, u, req) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*linkv1beta1.PublicShare) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *userv1beta1.User, *linkv1beta1.UpdatePublicShareRequest) error); ok { + r1 = rf(ctx, u, req) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Manager_UpdatePublicShare_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpdatePublicShare' +type Manager_UpdatePublicShare_Call struct { + *mock.Call +} + +// UpdatePublicShare is a helper method to define mock.On call +// - ctx context.Context +// - u *userv1beta1.User +// - req *linkv1beta1.UpdatePublicShareRequest +func (_e *Manager_Expecter) UpdatePublicShare(ctx interface{}, u interface{}, req interface{}) *Manager_UpdatePublicShare_Call { + return &Manager_UpdatePublicShare_Call{Call: _e.mock.On("UpdatePublicShare", ctx, u, req)} +} + +func (_c *Manager_UpdatePublicShare_Call) Run(run func(ctx context.Context, u *userv1beta1.User, req *linkv1beta1.UpdatePublicShareRequest)) *Manager_UpdatePublicShare_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*userv1beta1.User), args[2].(*linkv1beta1.UpdatePublicShareRequest)) + }) + return _c +} + +func (_c *Manager_UpdatePublicShare_Call) Return(_a0 *linkv1beta1.PublicShare, _a1 error) *Manager_UpdatePublicShare_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *Manager_UpdatePublicShare_Call) RunAndReturn(run func(context.Context, *userv1beta1.User, *linkv1beta1.UpdatePublicShareRequest) (*linkv1beta1.PublicShare, error)) *Manager_UpdatePublicShare_Call { + _c.Call.Return(run) + return _c +} + +// NewManager creates a new instance of Manager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewManager(t interface { + mock.TestingT + Cleanup(func()) +}) *Manager { + mock := &Manager{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 2d688a29626b5471f97f6f24c0b42afd868c8d23 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 7 May 2024 20:01:13 +0200 Subject: [PATCH 2/4] fix: always permit to create internal links --- .../publicshareprovider.go | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index 4e58661916..7876750b67 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -236,7 +236,7 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS } // check that user has share permissions - if !sRes.GetInfo().GetPermissionSet().AddGrant { + if !isInternalLink && !sRes.GetInfo().GetPermissionSet().AddGrant { return &link.CreatePublicShareResponse{ Status: status.NewInvalidArg(ctx, "no share permission"), }, nil @@ -480,14 +480,12 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS isInternalLink := isInternalLink(req, ps) - // users should always be able to downgrade links to internal links - // when they are the creator of the link - // all other users should have the WritePublicLink permission - if !isInternalLink && !publicshare.IsCreatedByUser(ps, user) { + // check if the user has the permission in the user role + if !publicshare.IsCreatedByUser(ps, user) { canWriteLink, err := utils.CheckPermission(ctx, permission.WritePublicLink, gatewayClient) if err != nil { return &link.UpdatePublicShareResponse{ - Status: status.NewInternal(ctx, "error loading public share"), + Status: status.NewInternal(ctx, "error checking permission to write public share"), }, err } if !canWriteLink { @@ -504,8 +502,14 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS Status: status.NewInternal(ctx, "failed to stat shared resource"), }, err } + if sRes.Status.Code != rpc.Code_CODE_OK { + return &link.UpdatePublicShareResponse{ + Status: sRes.GetStatus(), + }, nil - if !publicshare.IsCreatedByUser(ps, user) { + } + + if !isInternalLink && !publicshare.IsCreatedByUser(ps, user) { if !sRes.GetInfo().GetPermissionSet().UpdateGrant { return &link.UpdatePublicShareResponse{ Status: status.NewPermissionDenied(ctx, nil, "no permission to update public share"), @@ -550,12 +554,16 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS } // enforce password if needed - canOptOut, err := utils.CheckPermission(ctx, permission.DeleteReadOnlyPassword, gatewayClient) - if err != nil { - return &link.UpdatePublicShareResponse{ - Status: status.NewInternal(ctx, err.Error()), - }, nil + var canOptOut bool + if !isInternalLink { + canOptOut, err = utils.CheckPermission(ctx, permission.DeleteReadOnlyPassword, gatewayClient) + if err != nil { + return &link.UpdatePublicShareResponse{ + Status: status.NewInternal(ctx, err.Error()), + }, nil + } } + updatePassword := req.GetUpdate().GetType() == link.UpdatePublicShareRequest_Update_TYPE_PASSWORD setPassword := grant.GetPassword() From a971c50e2d28e616ea3a093e3a5f8bbb814d19f8 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 7 May 2024 22:14:42 +0200 Subject: [PATCH 3/4] tests: unit tests for update and get public shares --- .../publicshareprovider_test.go | 788 +++++++++++++++++- 1 file changed, 770 insertions(+), 18 deletions(-) diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider_test.go b/internal/grpc/services/publicshareprovider/publicshareprovider_test.go index 3d63353147..5534b1628d 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider_test.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider_test.go @@ -12,6 +12,8 @@ import ( providerpb "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/internal/grpc/services/publicshareprovider" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/errtypes" + "github.com/cs3org/reva/v2/pkg/permission" "github.com/cs3org/reva/v2/pkg/publicshare" "github.com/cs3org/reva/v2/pkg/publicshare/manager/registry" "github.com/cs3org/reva/v2/pkg/publicshare/mocks" @@ -130,14 +132,6 @@ var _ = Describe("PublicShareProvider", func() { InitiateFileUpload: true, } - createdLink = &link.PublicShare{ - PasswordProtected: true, - Permissions: &link.PublicSharePermissions{ - Permissions: linkPermissions, - }, - Creator: user.Id, - } - revaConfig = map[string]interface{}{ "driver": "mockManager", "drivers": map[string]map[string]interface{}{ @@ -182,6 +176,14 @@ var _ = Describe("PublicShareProvider", func() { EXPECT(). Stat(mock.Anything, mock.Anything). Return(statResourceResponse, nil) + + createdLink = &link.PublicShare{ + PasswordProtected: true, + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + Creator: user.Id, + } }) It("creates a public share with password", func() { manager. @@ -260,9 +262,13 @@ var _ = Describe("PublicShareProvider", func() { Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) Expect(res.GetShare()).To(Equal(createdLink)) }) - It("has no share permission on the resource to create internal link", func() { + It("has no share permission on the resource, can create internal link", func() { // internal links are created with empty permissions linkPermissions := &providerpb.ResourcePermissions{} + manager. + EXPECT(). + CreatePublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(createdLink, nil) // internal link creation should not check user permissions gatewayClient.EXPECT().CheckPermission(mock.Anything, mock.Anything).Unset() @@ -287,8 +293,8 @@ var _ = Describe("PublicShareProvider", func() { res, err := provider.CreatePublicShare(ctx, req) Expect(err).ToNot(HaveOccurred()) - Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) - Expect(res.GetStatus().GetMessage()).To(Equal("no share permission")) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(createdLink)) }) It("fails to check create public share user permission", func() { gatewayClient.EXPECT().CheckPermission(mock.Anything, mock.Anything).Unset() @@ -427,12 +433,6 @@ var _ = Describe("PublicShareProvider", func() { }, ArbitraryMetadata: &providerpb.ArbitraryMetadata{Metadata: map[string]string{"quicklink": "true"}}, Path: "./NewFolder/file.txt", - Id: &providerpb.ResourceId{ - StorageId: "storage-id", - OpaqueId: "project-id", - SpaceId: "project-id", - }, - Space: &providerpb.StorageSpace{SpaceType: "project"}, }, Grant: &link.Grant{ Permissions: &link.PublicSharePermissions{ @@ -531,7 +531,7 @@ var _ = Describe("PublicShareProvider", func() { Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) Expect(res.GetStatus().GetMessage()).To(ContainSubstring("expiration date is in the past")) }) - It("create public share with valid expiration", func() { + It("create public share with valid expiration date", func() { tomorrow := time.Now().AddDate(0, 0, 1) tsTomorrow := utils.TimeToTS(tomorrow) @@ -921,4 +921,756 @@ var _ = Describe("PublicShareProvider", func() { }) }) }) + Describe("Updating a PublicShare", func() { + var ( + existingLink *link.PublicShare + updatedLink *link.PublicShare + ) + + BeforeEach(func() { + // stat the shared resource to get the users resource permissions + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(statResourceResponse, nil) + + existingLink = &link.PublicShare{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + Token: "token", + Permissions: &link.PublicSharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + AddGrant: true, + }, + }, + Creator: user.GetId(), + } + }) + Context("succeeds when the user downgrades a public link to internal", func() { + BeforeEach(func() { + linkPermissions = &providerpb.ResourcePermissions{} + updatedLink = &link.PublicShare{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + DisplayName: "Updated Link", + } + }) + It("fails when it cannot load the existing share", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + }) + It("fails when it cannot connect to the storage provider", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).To(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + }) + It("fails when it cannot find the shared resource", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + + statResourceResponse.Status = status.NewNotFound(context.TODO(), "not found") + statResourceResponse.Info = nil + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Return(statResourceResponse, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_NOT_FOUND)) + }) + It("fails when it cannot store share information", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + manager. + EXPECT(). + UpdatePublicShare(mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errors.New("storage error")) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("storage error")) + }) + It("fails when the user is neither the creator nor the owner of the share", func() { + existingLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + checkPermissionResponse.Status.Code = rpc.Code_CODE_PERMISSION_DENIED + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(checkPermissionResponse, nil) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + + }) + It("fails when user is neither the resource owner nor the share creator", func() { + existingLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + checkPermissionResponse.Status.Code = rpc.Code_CODE_PERMISSION_DENIED + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(checkPermissionResponse, nil) + + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + + linkPermissions.Delete = true + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + + }) + It("succeeds", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + manager. + EXPECT(). + UpdatePublicShare(mock.Anything, mock.Anything, mock.Anything). + Once().Return(updatedLink, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(updatedLink)) + }) + It("succeeds even if the user is no manager or owner on the resource", func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + manager. + EXPECT(). + UpdatePublicShare(mock.Anything, mock.Anything, mock.Anything). + Once().Return(updatedLink, nil) + + statResourceResponse.Info.PermissionSet.UpdateGrant = false + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(updatedLink)) + }) + }) + Context("when the user changes permissions", func() { + BeforeEach(func() { + linkPermissions = &providerpb.ResourcePermissions{ + Stat: true, + AddGrant: true, + Delete: true, + Move: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + CreateContainer: true, + ListContainer: true, + ListGrants: true, + } + + updatedLink = &link.PublicShare{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + DisplayName: "Updated Link", + } + + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + }) + It("fails when the user has not enough permissions on the resource", func() { + statResourceResponse.Info.PermissionSet.Delete = false + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(Equal("insufficient permissions to update that kind of share")) + }) + It("fails when the permissions client is not responding", func() { + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(nil, errors.New("transport error")) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("transport error")) + }) + It("fails when the user has no permission to write public shares and is not the creator", func() { + checkPermissionResponse.Status.Code = rpc.Code_CODE_PERMISSION_DENIED + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(checkPermissionResponse, nil) + + existingLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + gatewayClient. + EXPECT(). + Stat(mock.Anything, mock.Anything). + Unset() + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + Expect(res.GetStatus().GetMessage()).To(Equal("no permission to update public share")) + }) + It("fails when the user is not the creator and has no manager or owner permissions", func() { + existingLink.Creator = &userpb.UserId{ + OpaqueId: "admin", + } + + gatewayClient. + EXPECT(). + CheckPermission(mock.Anything, mock.Anything). + Return(checkPermissionResponse, nil) + + statResourceResponse.Info.PermissionSet.UpdateGrant = false + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: linkPermissions, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + Expect(res.GetStatus().GetMessage()).To(Equal("no permission to update public share")) + }) + It("fails when the expiration date is in the past", func() { + yesterday := time.Now().AddDate(0, 0, -1) + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_EXPIRATION, + Grant: &link.Grant{ + Expiration: utils.TimeToTS(yesterday), + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("expiration date is in the past")) + }) + It("fails when the password is changed to on a writable share", func() { + gatewayClient. + EXPECT(). + CheckPermission( + mock.Anything, + mock.MatchedBy( + func(req *permissions.CheckPermissionRequest) bool { + return req.GetPermission() == permission.DeleteReadOnlyPassword + }, + ), + ). + Return(checkPermissionResponse, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PASSWORD, + Grant: &link.Grant{ + Password: "", + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("password protection is enforced")) + }) + It("fails when the password is already empty on a writable share", func() { + gatewayClient. + EXPECT(). + CheckPermission( + mock.Anything, + mock.MatchedBy( + func(req *permissions.CheckPermissionRequest) bool { + return req.GetPermission() == permission.DeleteReadOnlyPassword + }, + ), + ). + Return(checkPermissionResponse, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + Move: true, + }, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("password protection is enforced")) + }) + It("succeeds when the password is empty on a readable share with opt out permission", func() { + gatewayClient. + EXPECT(). + CheckPermission( + mock.Anything, + mock.MatchedBy( + func(req *permissions.CheckPermissionRequest) bool { + return req.GetPermission() == permission.DeleteReadOnlyPassword + }, + ), + ). + Return(checkPermissionResponse, nil) + + manager. + EXPECT(). + UpdatePublicShare(mock.Anything, mock.Anything, mock.Anything). + Once().Return(updatedLink, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, + Grant: &link.Grant{ + Permissions: &link.PublicSharePermissions{ + Permissions: &providerpb.ResourcePermissions{ + Stat: true, + ListContainer: true, + InitiateFileDownload: true, + }, + }, + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_OK)) + Expect(res.GetShare()).To(Equal(updatedLink)) + }) + It("fails when the updated password doesn't fulfil the policy", func() { + gatewayClient. + EXPECT(). + CheckPermission( + mock.Anything, + mock.MatchedBy( + func(req *permissions.CheckPermissionRequest) bool { + return req.GetPermission() == permission.DeleteReadOnlyPassword + }, + ), + ). + Return(checkPermissionResponse, nil) + + req := &link.UpdatePublicShareRequest{ + Update: &link.UpdatePublicShareRequest_Update{ + Type: link.UpdatePublicShareRequest_Update_TYPE_PASSWORD, + Grant: &link.Grant{ + Password: "Test", + }, + }, + } + + res, err := provider.UpdatePublicShare(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("characters are required")) + }) + }) + Context("when the user gets a public share", func() { + BeforeEach(func() { + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything).Unset() + }) + It("succeeds", func() { + res, err := provider.GetPublicShare(ctx, &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetShare()).To(Equal(existingLink)) + }) + It("fails when it finds no share", func() { + manager.EXPECT().GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errtypes.NotFound("not found")) + res, err := provider.GetPublicShare(ctx, &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_NOT_FOUND)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("not found")) + }) + It("fails when it finds no share due to internal error", func() { + manager.EXPECT().GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errtypes.InternalError("internal error")) + res, err := provider.GetPublicShare(ctx, &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("internal error")) + }) + It("fails when the share manager response is empty", func() { + manager.EXPECT().GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShare(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, nil) + res, err := provider.GetPublicShare(ctx, &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: "share-id", + }, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_NOT_FOUND)) + Expect(res.GetStatus().GetMessage()).To(ContainSubstring("not found")) + }) + }) + Context("when the user gets a public share by token", func() { + BeforeEach(func() { + manager. + EXPECT(). + GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(existingLink, nil) + + gatewayClient.EXPECT().Stat(mock.Anything, mock.Anything).Unset() + }) + It("succeeds", func() { + res, err := provider.GetPublicShareByToken(ctx, &link.GetPublicShareByTokenRequest{ + Token: "token", + Authentication: &link.PublicShareAuthentication{ + Spec: &link.PublicShareAuthentication_Password{ + Password: "password", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetShare()).To(Equal(existingLink)) + }) + It("fails with invalid credentials", func() { + manager.EXPECT().GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errtypes.InvalidCredentials("wrong password")) + + res, err := provider.GetPublicShareByToken(ctx, &link.GetPublicShareByTokenRequest{ + Token: "token", + Authentication: &link.PublicShareAuthentication{ + Spec: &link.PublicShareAuthentication_Password{ + Password: "password", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_PERMISSION_DENIED)) + Expect(res.GetStatus().GetMessage()).To(Equal("wrong password")) + }) + It("fails with an unknown token", func() { + manager.EXPECT().GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errtypes.NotFound("public share not found")) + + res, err := provider.GetPublicShareByToken(ctx, &link.GetPublicShareByTokenRequest{ + Token: "token", + Authentication: &link.PublicShareAuthentication{ + Spec: &link.PublicShareAuthentication_Password{ + Password: "password", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_NOT_FOUND)) + Expect(res.GetStatus().GetMessage()).To(Equal("unknown token")) + }) + It("fails with an unknown error", func() { + manager.EXPECT().GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset() + manager. + EXPECT(). + GetPublicShareByToken(mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Once().Return(nil, errtypes.InternalError("internal error")) + + res, err := provider.GetPublicShareByToken(ctx, &link.GetPublicShareByTokenRequest{ + Token: "token", + Authentication: &link.PublicShareAuthentication{ + Spec: &link.PublicShareAuthentication_Password{ + Password: "password", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res.GetStatus().GetCode()).To(Equal(rpc.Code_CODE_INTERNAL)) + Expect(res.GetStatus().GetMessage()).To(Equal("unexpected error")) + }) + }) + }) }) From be80467068f506bab48e9b10f1bf0ea7abfe9784 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 9 Jul 2024 15:40:30 +0200 Subject: [PATCH 4/4] docs: add changelog --- changelog/2.20.0_2024-06-19/fix-public-share-update.md | 3 +-- changelog/unreleased/fix-create-internal-links.md | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/fix-create-internal-links.md diff --git a/changelog/2.20.0_2024-06-19/fix-public-share-update.md b/changelog/2.20.0_2024-06-19/fix-public-share-update.md index acf5a1283e..0d00de2eab 100644 --- a/changelog/2.20.0_2024-06-19/fix-public-share-update.md +++ b/changelog/2.20.0_2024-06-19/fix-public-share-update.md @@ -2,5 +2,4 @@ Bugfix: Fix public share update We fixed the permission check for updating public shares. When updating the permissions of a public share while not providing a password, the check must be against the new permissions to take into account that users can opt out only for view permissions. -https://github.com/cs3org/reva/pull/4633 -https://github.com/cs3org/reva/pull/4622 \ No newline at end of file +https://github.com/cs3org/reva/pull/4622 diff --git a/changelog/unreleased/fix-create-internal-links.md b/changelog/unreleased/fix-create-internal-links.md new file mode 100644 index 0000000000..33e68c0ed6 --- /dev/null +++ b/changelog/unreleased/fix-create-internal-links.md @@ -0,0 +1,6 @@ +Bugfix: Allow all users to create internal links + +Due to a bug, not all space members were allowed to create internal links. This has been fixed. + +https://github.com/cs3org/reva/pull/4633 +https://github.com/owncloud/ocis/issues/8960