Add "delete-all-spaces" permission check

We introduced a new permission "delete-all-spaces", users holding this permission are allowed to delete any space of any type.
cs3org · Sep 7, 2022 · fbda29e · fbda29e
1 parent bb524f7
commit fbda29e
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 31 deletions.
diff --git a/changelog/unreleased/delete-app-spaces-perm.md 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
@@ -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
@@ -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
@@ -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)