Prevent space managers from purging spaces

pkg/storage/fs/posix/tree/tree.go

@@ -850,6 +850,7 @@ func (t *Tree) createDirNode(ctx context.Context, n *node.Node) (err error) {
 	attributes := n.NodeMetadata(ctx)
 	attributes[prefixes.MTimeAttr] = []byte(mtime.UTC().Format(time.RFC3339Nano))
 	attributes[prefixes.IDAttr] = []byte(n.ID)
+	attributes[prefixes.SpaceIDAttr] = []byte(n.SpaceID)
 	attributes[prefixes.TreesizeAttr] = []byte("0") // initialize as empty, TODO why bother? if it is not set we could treat it as 0?
 
 	if t.options.TreeTimeAccounting || t.options.TreeSizeAccounting {

pkg/storage/pkg/decomposedfs/decomposedfs_concurrency_test.go

@@ -178,7 +178,7 @@ var _ = Describe("Decomposed", func() {
 
 		Describe("CreateDir", func() {
 			JustBeforeEach(func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 					Stat:            true,
 					CreateContainer: true,
 				}, nil)

pkg/storage/pkg/decomposedfs/decomposedfs_test.go

@@ -73,7 +73,7 @@ var _ = Describe("Decomposed", func() {
 					ResourceId: env.SpaceRootRes,
 					Path:       "/dir2",
 				}
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true, Stat: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true, Stat: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, dir2)
 				Expect(err).ToNot(HaveOccurred())
 				ri, err := env.Fs.GetMD(env.Ctx, dir2, []string{}, []string{})
@@ -85,15 +85,15 @@ var _ = Describe("Decomposed", func() {
 					ResourceId: env.SpaceRootRes,
 					Path:       "/dir1/dir2",
 				}
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true, Stat: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true, Stat: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, dir2)
 				Expect(err).ToNot(HaveOccurred())
 				ri, err := env.Fs.GetMD(env.Ctx, dir2, []string{}, []string{})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(ri.Path).To(Equal(dir2.Path))
 			})
 			It("dir already exists", func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, ref)
 				Expect(err).To(HaveOccurred())
 				Expect(err).Should(MatchError(errtypes.AlreadyExists("/dir1")))
@@ -103,7 +103,7 @@ var _ = Describe("Decomposed", func() {
 					ResourceId: env.SpaceRootRes,
 					Path:       "/dir1/dir3",
 				}
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, dir3)
 				Expect(err).ToNot(HaveOccurred())
 				err = env.Fs.CreateDir(env.Ctx, dir3)
@@ -115,7 +115,7 @@ var _ = Describe("Decomposed", func() {
 					ResourceId: env.SpaceRootRes,
 					Path:       "/dir1/dir2/dir3",
 				}
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, dir2)
 				Expect(err).To(HaveOccurred())
 				Expect(err).Should(MatchError(errtypes.PreconditionFailed("/dir1/dir2")))
@@ -125,7 +125,7 @@ var _ = Describe("Decomposed", func() {
 					ResourceId: env.SpaceRootRes,
 					Path:       "/dir1/dir2/dir3/dir4",
 				}
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{CreateContainer: true}, nil)
 				err := env.Fs.CreateDir(env.Ctx, dir2)
 				Expect(err).To(HaveOccurred())
 				Expect(err).Should(MatchError(errtypes.PreconditionFailed("error: not found: dir2")))
@@ -136,7 +136,8 @@ var _ = Describe("Decomposed", func() {
 	Describe("Delete", func() {
 		Context("with no permissions", func() {
 			It("returns an error", func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{}, nil)
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{}, nil)
 
 				err := env.Fs.Delete(env.Ctx, ref)
 
@@ -146,7 +147,8 @@ var _ = Describe("Decomposed", func() {
 
 		Context("with insufficient permissions", func() {
 			It("returns an error", func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 					Stat:   true,
 					Delete: false,
 				}, nil)
@@ -159,7 +161,8 @@ var _ = Describe("Decomposed", func() {
 
 		Context("with sufficient permissions", func() {
 			JustBeforeEach(func() {
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 					Stat:   true,
 					Delete: true,
 				}, nil)

pkg/storage/pkg/decomposedfs/grants_test.go

@@ -81,7 +81,8 @@ var _ = Describe("Grants", func() {
 
 	Context("with no permissions", func() {
 		JustBeforeEach(func() {
-			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{}, nil)
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{}, nil)
 		})
 
 		Describe("AddGrant", func() {
@@ -94,7 +95,8 @@ var _ = Describe("Grants", func() {
 
 	Context("with insufficient permissions", func() {
 		JustBeforeEach(func() {
-			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 				Stat: true,
 			}, nil)
 		})
@@ -109,7 +111,7 @@ var _ = Describe("Grants", func() {
 
 	Context("with sufficient permissions", func() {
 		JustBeforeEach(func() {
-			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 				Stat:        true,
 				AddGrant:    true,
 				ListGrants:  true,

pkg/storage/pkg/decomposedfs/recycle_test.go

@@ -134,8 +134,13 @@ var _ = Describe("Recycle", func() {
 						Username: "anotherusername",
 					})
 
+					// Clear the initial mock setup
+					env.GetPermissions().On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+					env.GetPermissions().On("AssembleTrashPermissions", mock.Anything, mock.Anything).Unset()
+
 					// in this scenario user "25b69780-5f39-43be-a7ac-a9b9e9fe4230" has this permissions:
 					registerPermissions(env.GetPermissions(), "25b69780-5f39-43be-a7ac-a9b9e9fe4230", &provider.ResourcePermissions{
+						Stat:               true,
 						InitiateFileUpload: true,
 						Delete:             true,
 						ListRecycle:        true,
@@ -145,6 +150,7 @@ var _ = Describe("Recycle", func() {
 
 					// and user "anotheruserid" has the same permissions:
 					registerPermissions(env.GetPermissions(), "anotheruserid", &provider.ResourcePermissions{
+						Stat:               true,
 						InitiateFileUpload: true,
 						Delete:             true,
 						ListRecycle:        true,
@@ -318,6 +324,10 @@ var _ = Describe("Recycle", func() {
 						Username: "readusername",
 					})
 
+					// Clear the initial mock setup
+					env.GetPermissions().On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+					env.GetPermissions().On("AssembleTrashPermissions", mock.Anything, mock.Anything).Unset()
+
 					// in this scenario user "25b69780-5f39-43be-a7ac-a9b9e9fe4230" has this permissions:
 					registerPermissions(env.GetPermissions(), "25b69780-5f39-43be-a7ac-a9b9e9fe4230", &provider.ResourcePermissions{
 						Stat:               true,

pkg/storage/pkg/decomposedfs/spaces.go

@@ -1147,8 +1147,8 @@ func canDeleteSpace(ctx context.Context, spaceID string, typ string, purge bool,
 		return errtypes.PermissionDenied("user is not allowed to delete a personal space")
 	}
 
-	// space managers are allowed to disable and delete their project spaces
-	if rp, err := p.AssemblePermissions(ctx, n); err == nil && permissions.IsManager(rp) {
+	// space managers are allowed to disable their project spaces
+	if rp, err := p.AssemblePermissions(ctx, n); err == nil && !purge && permissions.IsManager(rp) {
 		return nil
 	}
 

pkg/storage/pkg/decomposedfs/spaces_test.go

@@ -59,12 +59,6 @@ var _ = Describe("Spaces", func() {
 					return &cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_PERMISSION_DENIED}}
 				},
 				nil)
-			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(func(ctx context.Context, n *node.Node) *provider.ResourcePermissions {
-				if ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId() == "25b69780-5f39-43be-a7ac-a9b9e9fe4230" {
-					return node.OwnerPermissions() // id of owner/admin
-				}
-				return node.NoPermissions()
-			}, nil)
 		})
 
 		AfterEach(func() {
@@ -85,7 +79,6 @@ var _ = Describe("Spaces", func() {
 		})
 		Context("when creating a space", func() {
 			It("project space is created", func() {
-				env.Owner = nil
 				resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Mission to Mars", Type: "project"})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(resp.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
@@ -198,9 +191,10 @@ var _ = Describe("Spaces", func() {
 				err := env.Fs.DeleteStorageSpace(ctx, delReq)
 				Expect(err).To(Not(HaveOccurred()))
 			})
-			It("succeeds as the space owner", func() {
-				err := env.Fs.DeleteStorageSpace(env.Ctx, delReq)
-				Expect(err).To(Not(HaveOccurred()))
+			It("fails as a space manager", func() {
+				ctx := ctxpkg.ContextSetUser(context.Background(), env.SpaceManager)
+				err := env.Fs.DeleteStorageSpace(ctx, delReq)
+				Expect(err).To(HaveOccurred())
 			})
 		})
 	})
@@ -219,7 +213,7 @@ var _ = Describe("Spaces", func() {
 				})
 				Expect(err).ToNot(HaveOccurred())
 				env.PermissionsClient.On("CheckPermission", mock.Anything, mock.Anything, mock.Anything).Return(&cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}}, nil)
-				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
+				env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{
 					Stat:     true,
 					AddGrant: true,
 					GetQuota: true,
@@ -241,7 +235,6 @@ var _ = Describe("Spaces", func() {
 			})
 			Context("creating a space", func() {
 				It("project space is created with custom alias", func() {
-					env.Owner = nil
 					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))
@@ -287,7 +280,8 @@ var _ = Describe("Spaces", func() {
 					}
 				}, nil)
 
-			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset()
+			env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(
 				func(ctx context.Context, n *node.Node) *provider.ResourcePermissions {
 					switch ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId() {
 					case manager.GetId().GetOpaqueId():

pkg/storage/pkg/decomposedfs/testhelpers/helpers.go

@@ -26,6 +26,8 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	conv "github.com/opencloud-eu/reva/v2/pkg/conversions"
+	ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx"
 	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/opencloud-eu/reva/v2/pkg/storage"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/timemanager"
@@ -44,7 +46,6 @@ import (
 	cs3permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1"
 	v1beta11 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	ruser "github.com/opencloud-eu/reva/v2/pkg/ctx"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs"
 	"github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node"
 	nodemocks "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node/mocks"
@@ -73,6 +74,7 @@ type DecomposedTestEnv struct {
 	Permissions          *mocks.PermissionsChecker
 	Blobstore            *nodemocks.Blobstore
 	Owner                *userpb.User
+	SpaceManager         *userpb.User
 	DeleteAllSpacesUser  *userpb.User
 	DeleteHomeSpacesUser *userpb.User
 	Users                []*userpb.User
@@ -108,6 +110,7 @@ const (
 	OwnerID                = "25b69780-5f39-43be-a7ac-a9b9e9fe4230"
 	DeleteAllSpacesUserID  = "39885dbc-68c0-47c0-a873-9d5e5646dceb"
 	DeleteHomeSpacesUserID = "ca8c6bf1-36a7-4d10-87a5-a2806566f983"
+	ManagerID              = "c02cb356-9df3-429d-9e05-b74c14117a78"
 	User0ID                = "824385ae-8fc6-4896-8eb2-d1d171290bd0"
 	User1ID                = "693b0d96-80a2-4016-b53d-425ce4f66114"
 )
@@ -169,6 +172,14 @@ func NewTestEnv(config map[string]interface{}) (*DecomposedTestEnv, error) {
 		},
 		Username: "username",
 	}
+	spaceManager := &userpb.User{
+		Id: &userpb.UserId{
+			Idp:      "idp",
+			OpaqueId: ManagerID,
+			Type:     userpb.UserType_USER_TYPE_PRIMARY,
+		},
+		Username: "manager",
+	}
 	users := []*userpb.User{
 		{
 			Id: &userpb.UserId{
@@ -220,7 +231,7 @@ func NewTestEnv(config map[string]interface{}) (*DecomposedTestEnv, error) {
 	if err != nil {
 		return nil, err
 	}
-	ctx := ruser.ContextSetUser(context.Background(), owner)
+	ctx := ctxpkg.ContextSetUser(context.Background(), owner)
 
 	tmpFs, _ := fs.(*decomposedfs.Decomposedfs)
 
@@ -232,6 +243,7 @@ func NewTestEnv(config map[string]interface{}) (*DecomposedTestEnv, error) {
 		Permissions:          pmock,
 		Blobstore:            bs,
 		Owner:                owner,
+		SpaceManager:         spaceManager,
 		DeleteAllSpacesUser:  deleteAllSpacesUser,
 		DeleteHomeSpacesUser: deleteHomeSpacesUser,
 		Users:                users,
@@ -325,10 +337,17 @@ func (t *DecomposedTestEnv) CreateTestStorageSpace(typ string, quota *providerv1
 		Status: &v1beta11.Status{Code: v1beta11.Code_CODE_OK},
 	}, nil)
 	// Permissions required for setup below
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
-		Stat:     true,
-		AddGrant: true,
-	}, nil).Times(1) //
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(
+		func(ctx context.Context, n *node.Node) (*providerv1beta1.ResourcePermissions, error) {
+			id := ctxpkg.ContextMustGetUser(ctx).Id.GetOpaqueId()
+			switch id {
+			case t.Owner.GetId().GetOpaqueId(): // owner/admin
+				return node.OwnerPermissions(), nil
+			case t.SpaceManager.GetId().GetOpaqueId(): // space manager
+				return conv.NewManagerRole().CS3ResourcePermissions(), nil
+			}
+			return node.NoPermissions(), nil
+		}, nil)
 
 	var owner *userpb.User
 	if typ == "personal" {
@@ -343,6 +362,21 @@ func (t *DecomposedTestEnv) CreateTestStorageSpace(typ string, quota *providerv1
 		return nil, err
 	}
 
+	err = t.Fs.AddGrant(t.Ctx, &providerv1beta1.Reference{
+		ResourceId: space.StorageSpace.Root,
+	}, &providerv1beta1.Grant{
+		Grantee: &providerv1beta1.Grantee{
+			Type: providerv1beta1.GranteeType_GRANTEE_TYPE_USER,
+			Id: &providerv1beta1.Grantee_UserId{
+				UserId: t.SpaceManager.Id,
+			},
+		},
+		Permissions: conv.NewManagerRole().CS3ResourcePermissions(),
+	})
+	if err != nil {
+		return nil, err
+	}
+
 	ref := buildRef(space.StorageSpace.Id.OpaqueId, "")
 
 	// the space name attribute is the stop condition in the lookup
@@ -359,7 +393,7 @@ func (t *DecomposedTestEnv) CreateTestStorageSpace(typ string, quota *providerv1
 	}
 
 	// Create dir1
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below
@@ -375,7 +409,7 @@ func (t *DecomposedTestEnv) CreateTestStorageSpace(typ string, quota *providerv1
 	}
 
 	// Create subdir1 in dir1
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below
@@ -391,7 +425,7 @@ func (t *DecomposedTestEnv) CreateTestStorageSpace(typ string, quota *providerv1
 	}
 
 	// Create emptydir
-	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
+	t.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&providerv1beta1.ResourcePermissions{
 		Stat:            true,
 		CreateContainer: true,
 	}, nil).Times(1) // Permissions required for setup below

pkg/storage/pkg/decomposedfs/tree/tree.go

@@ -845,6 +845,7 @@ func (t *Tree) createDirNode(ctx context.Context, n *node.Node) (err error) {
 	}
 
 	attributes := n.NodeMetadata(ctx)
+	attributes[prefixes.SpaceIDAttr] = []byte(n.SpaceID)
 	attributes[prefixes.TreesizeAttr] = []byte("0") // initialize as empty, TODO why bother? if it is not set we could treat it as 0?
 	if t.options.TreeTimeAccounting || t.options.TreeSizeAccounting {
 		attributes[prefixes.PropagationAttr] = []byte("1") // mark the node for propagation