diff --git a/changelog/unreleased/delete-app-spaces-perm.md b/changelog/unreleased/delete-app-spaces-perm.md new file mode 100644 index 00000000000..4b56f407d0b --- /dev/null +++ b/changelog/unreleased/delete-app-spaces-perm.md @@ -0,0 +1,6 @@ +Enhancement: Added "delete-all-spaces" permission + +We introduced a new permission "delete-all-spaces", user holding this permission are allowed +to delete any space of any type. + +https://github.com/cs3org/reva/pull/3xxx diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index 3d6dfaac9ba..3a9457dcd24 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -196,10 +196,10 @@ func (fs *Decomposedfs) canListAllSpaces(ctx context.Context) bool { return checkRes.Status.Code == v1beta11.Code_CODE_OK } -func (fs *Decomposedfs) canDeleteAllHomeSpaces(ctx context.Context) bool { +func (fs *Decomposedfs) checkSpacePermission(ctx context.Context, permission string) bool { user := ctxpkg.ContextMustGetUser(ctx) checkRes, err := fs.permissionsClient.CheckPermission(ctx, &cs3permissions.CheckPermissionRequest{ - Permission: "delete-all-home-spaces", + Permission: permission, SubjectRef: &cs3permissions.SubjectReference{ Spec: &cs3permissions.SubjectReference_UserId{ UserId: user.Id, @@ -213,6 +213,14 @@ func (fs *Decomposedfs) canDeleteAllHomeSpaces(ctx context.Context) bool { return checkRes.Status.Code == v1beta11.Code_CODE_OK } +func (fs *Decomposedfs) canDeleteAllSpaces(ctx context.Context) bool { + return fs.checkSpacePermission(ctx, "delete-all-spaces") +} + +func (fs *Decomposedfs) canDeleteAllHomeSpaces(ctx context.Context) bool { + return fs.checkSpacePermission(ctx, "delete-all-home-spaces") +} + // returns true when the user in the context can create a space / resource with storageID and nodeID set to his user opaqueID func (fs *Decomposedfs) canCreateSpace(ctx context.Context, spaceID string) bool { user := ctxpkg.ContextMustGetUser(ctx) @@ -635,7 +643,13 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De return errtypes.InternalError(fmt.Sprintf("space %s does not have a spacetype, possible corrupt decompsedfs", n.ID)) } + // - a User with the "delete-all-spaces" permission can delete any space + // - spaces of type personal can also be delete by user with the "delete-all-home-spaces" permission + // - otherwise a space can be deleted by it's manager (i.e. users have the "remove" grant) switch { + case fs.canDeleteAllSpaces(ctx): + // We are allowed to delete any space, no further permission checks needed + break case st == "personal": if !fs.canDeleteAllHomeSpaces(ctx) { return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete home space %s", n.ID)) diff --git a/pkg/storage/utils/decomposedfs/spaces_test.go b/pkg/storage/utils/decomposedfs/spaces_test.go index 61968a0f216..66cd8ea9b59 100644 --- a/pkg/storage/utils/decomposedfs/spaces_test.go +++ b/pkg/storage/utils/decomposedfs/spaces_test.go @@ -50,11 +50,16 @@ var _ = Describe("Spaces", func() { Expect(err).ToNot(HaveOccurred()) env.PermissionsClient.On("CheckPermission", mock.Anything, mock.Anything, mock.Anything).Return( func(ctx context.Context, in *cs3permissions.CheckPermissionRequest, opts ...grpc.CallOption) *cs3permissions.CheckPermissionResponse { - if ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId() == "25b69780-5f39-43be-a7ac-a9b9e9fe4230" { - // id of owner/admin + if in.Permission == "delete-all-home-spaces" && ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId() == env.DeleteHomeSpacesUser.Id.OpaqueId { return &cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}} } - // id of generic user + if in.Permission == "delete-all-spaces" && ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId() == env.DeleteAllSpacesUser.Id.OpaqueId { + return &cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}} + } + if (in.Permission == "create-space" || in.Permission == "list-all-spaces") && ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId() == helpers.OwnerID { + return &cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}} + } + // any other user return &cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_PERMISSION_DENIED}} }, nil) @@ -153,8 +158,21 @@ var _ = Describe("Spaces", func() { }) Expect(err).To(HaveOccurred()) }) - It("succeeds on trying to delete homespace as admin", func() { - ctx := ctxpkg.ContextSetUser(context.Background(), env.Owner) + It("succeeds on trying to delete homespace as user with 'delete-all-home-spaces' permission", func() { + ctx := ctxpkg.ContextSetUser(context.Background(), env.DeleteHomeSpacesUser) + resp, err := env.Fs.ListStorageSpaces(env.Ctx, nil, false) + Expect(err).ToNot(HaveOccurred()) + Expect(len(resp)).To(Equal(1)) + Expect(string(resp[0].Opaque.GetMap()["spaceAlias"].Value)).To(Equal("personal/username")) + Expect(resp[0].Name).To(Equal("username")) + Expect(resp[0].SpaceType).To(Equal("personal")) + err = env.Fs.DeleteStorageSpace(ctx, &provider.DeleteStorageSpaceRequest{ + Id: resp[0].GetId(), + }) + Expect(err).To(Not(HaveOccurred())) + }) + It("succeeds on trying to delete homespace as user with 'delete-all-spaces' permission", func() { + ctx := ctxpkg.ContextSetUser(context.Background(), env.DeleteAllSpacesUser) resp, err := env.Fs.ListStorageSpaces(env.Ctx, nil, false) Expect(err).ToNot(HaveOccurred()) Expect(len(resp)).To(Equal(1)) @@ -168,6 +186,52 @@ var _ = Describe("Spaces", func() { }) }) + Context("can delete project spaces", func() { + It("fails as a unprivileged user", func() { + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Venus", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + Expect(resp.StorageSpace).ToNot(Equal(nil)) + ctx := ctxpkg.ContextSetUser(context.Background(), env.Users[1]) + err = env.Fs.DeleteStorageSpace(ctx, &provider.DeleteStorageSpaceRequest{ + Id: resp.StorageSpace.GetId(), + }) + Expect(err).To(HaveOccurred()) + }) + It("fails as a user with 'delete-all-home-spaces privilege", func() { + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Venus", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + Expect(resp.StorageSpace).ToNot(Equal(nil)) + ctx := ctxpkg.ContextSetUser(context.Background(), env.DeleteHomeSpacesUser) + err = env.Fs.DeleteStorageSpace(ctx, &provider.DeleteStorageSpaceRequest{ + Id: resp.StorageSpace.GetId(), + }) + Expect(err).To(HaveOccurred()) + }) + It("succeeds as a user with 'delete-all-spaces privilege", func() { + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Venus", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + Expect(resp.StorageSpace).ToNot(Equal(nil)) + ctx := ctxpkg.ContextSetUser(context.Background(), env.DeleteAllSpacesUser) + err = env.Fs.DeleteStorageSpace(ctx, &provider.DeleteStorageSpaceRequest{ + Id: resp.StorageSpace.GetId(), + }) + Expect(err).To(Not(HaveOccurred())) + }) + It("succeeds as the space owner", func() { + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Venus", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) + Expect(resp.StorageSpace).ToNot(Equal(nil)) + err = env.Fs.DeleteStorageSpace(env.Ctx, &provider.DeleteStorageSpaceRequest{ + Id: resp.StorageSpace.GetId(), + }) + Expect(err).To(Not(HaveOccurred())) + }) + }) + Describe("Create Spaces with custom alias template", func() { var ( env *helpers.TestEnv diff --git a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go index 750f443c5e6..56937d30296 100644 --- a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go +++ b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go @@ -46,24 +46,28 @@ import ( // TestEnv represents a test environment for unit tests type TestEnv struct { - Root string - Fs *decomposedfs.Decomposedfs - Tree *tree.Tree - Permissions *mocks.PermissionsChecker - Blobstore *treemocks.Blobstore - Owner *userpb.User - Users []*userpb.User - Lookup *lookup.Lookup - Ctx context.Context - SpaceRootRes *providerv1beta1.ResourceId - PermissionsClient *mocks.CS3PermissionsClient + Root string + Fs *decomposedfs.Decomposedfs + Tree *tree.Tree + Permissions *mocks.PermissionsChecker + Blobstore *treemocks.Blobstore + Owner *userpb.User + DeleteAllSpacesUser *userpb.User + DeleteHomeSpacesUser *userpb.User + Users []*userpb.User + Lookup *lookup.Lookup + Ctx context.Context + SpaceRootRes *providerv1beta1.ResourceId + PermissionsClient *mocks.CS3PermissionsClient } // Constant UUIDs for the space users const ( - OwnerID = "25b69780-5f39-43be-a7ac-a9b9e9fe4230" - User0ID = "824385ae-8fc6-4896-8eb2-d1d171290bd0" - User1ID = "693b0d96-80a2-4016-b53d-425ce4f66114" + OwnerID = "25b69780-5f39-43be-a7ac-a9b9e9fe4230" + DeleteAllSpacesUserID = "39885dbc-68c0-47c0-a873-9d5e5646dceb" + DeleteHomeSpacesUserID = "ca8c6bf1-36a7-4d10-87a5-a2806566f983" + User0ID = "824385ae-8fc6-4896-8eb2-d1d171290bd0" + User1ID = "693b0d96-80a2-4016-b53d-425ce4f66114" ) // NewTestEnv prepares a test environment on disk @@ -105,6 +109,22 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) { }, Username: "username", } + deleteHomeSpacesUser := &userpb.User{ + Id: &userpb.UserId{ + Idp: "idp", + OpaqueId: DeleteHomeSpacesUserID, + Type: userpb.UserType_USER_TYPE_PRIMARY, + }, + Username: "username", + } + deleteAllSpacesUser := &userpb.User{ + Id: &userpb.UserId{ + Idp: "idp", + OpaqueId: DeleteAllSpacesUserID, + Type: userpb.UserType_USER_TYPE_PRIMARY, + }, + Username: "username", + } users := []*userpb.User{ { Id: &userpb.UserId{ @@ -135,16 +155,18 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) { tmpFs, _ := fs.(*decomposedfs.Decomposedfs) env := &TestEnv{ - Root: tmpRoot, - Fs: tmpFs, - Tree: tree, - Lookup: lookup, - Permissions: permissions, - Blobstore: bs, - Owner: owner, - Users: users, - Ctx: ctx, - PermissionsClient: cs3permissionsclient, + Root: tmpRoot, + Fs: tmpFs, + Tree: tree, + Lookup: lookup, + Permissions: permissions, + Blobstore: bs, + Owner: owner, + DeleteAllSpacesUser: deleteAllSpacesUser, + DeleteHomeSpacesUser: deleteHomeSpacesUser, + Users: users, + Ctx: ctx, + PermissionsClient: cs3permissionsclient, } env.SpaceRootRes, err = env.CreateTestStorageSpace("personal", nil)