fix(posix): check trash permissions in the posix trashbin

changelog/unreleased/bugfix-posix-trashbin-permissions.md

@@ -0,0 +1,9 @@
+Bugfix: Check permissions in the trashbin of the posix driver
+
+The trashbin of the posix storage driver now checks the trash permissions of
+the space on all recycle operations. Listing requires `ListRecycle`, restoring
+requires `RestoreRecycleItem`, and purging a single item or emptying the trash
+require `PurgeRecycle`. Note that a space viewer can therefore browse the trash
+but can no longer empty it. Only the posix driver is affected.
+
+https://github.com/owncloud/reva/pull/729

pkg/storage/fs/posix/posix.go

@@ -85,7 +85,13 @@ func New(m map[string]interface{}, stream events.Stream, log *zerolog.Logger) (s
 		return nil, fmt.Errorf("unknown metadata backend %s, only 'messagepack' or 'xattrs' (default) supported", o.MetadataBackend)
 	}
 
-	trashbin, err := trashbin.New(o, lu, log)
+	permissionsSelector, err := pool.PermissionsSelector(o.PermissionsSVC, pool.WithTLSMode(o.PermTLSMode))
+	if err != nil {
+		return nil, err
+	}
+	p := permissions.NewPermissions(node.NewPermissions(lu), permissionsSelector)
+
+	trashbin, err := trashbin.New(o, p, lu, log)
 	if err != nil {
 		return nil, err
 	}
@@ -119,13 +125,6 @@ func New(m map[string]interface{}, stream events.Stream, log *zerolog.Logger) (s
 		return nil, err
 	}
 
-	permissionsSelector, err := pool.PermissionsSelector(o.PermissionsSVC, pool.WithTLSMode(o.PermTLSMode))
-	if err != nil {
-		return nil, err
-	}
-
-	p := permissions.NewPermissions(node.NewPermissions(lu), permissionsSelector)
-
 	aspects := aspects.Aspects{
 		Lookup:            lu,
 		Tree:              tp,

pkg/storage/fs/posix/testhelpers/helpers.go

@@ -186,14 +186,15 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 	if err != nil {
 		return nil, err
 	}
-	tb, err := trashbin.New(o, lu, &logger)
+	p := permissions.NewPermissions(pmock, permissionsSelector)
+	tb, err := trashbin.New(o, p, lu, &logger)
 	if err != nil {
 		return nil, err
 	}
 	aspects := aspects.Aspects{
 		Lookup:      lu,
 		Tree:        tree,
-		Permissions: permissions.NewPermissions(pmock, permissionsSelector),
+		Permissions: p,
 		Trashbin:    tb,
 	}
 	fs, err := decomposedfs.New(&o.Options, aspects, &logger)

pkg/storage/fs/posix/trashbin/trashbin.go

@@ -31,6 +31,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/owncloud/reva/v2/pkg/errtypes"
 	"github.com/owncloud/reva/v2/pkg/storage"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/lookup"
 	"github.com/owncloud/reva/v2/pkg/storage/fs/posix/options"
@@ -42,6 +43,7 @@ import (
 type Trashbin struct {
 	fs  storage.FS
 	o   *options.Options
+	p   Permissions
 	lu  *lookup.Lookup
 	log *zerolog.Logger
 }
@@ -51,10 +53,16 @@ const (
 	timeFormat  = "2006-01-02T15:04:05"
 )
 
+// Permissions is the interface the trashbin uses to authorize recycle operations
+type Permissions interface {
+	AssembleTrashPermissions(ctx context.Context, n *node.Node) (*provider.ResourcePermissions, error)
+}
+
 // New returns a new Trashbin
-func New(o *options.Options, lu *lookup.Lookup, log *zerolog.Logger) (*Trashbin, error) {
+func New(o *options.Options, p Permissions, lu *lookup.Lookup, log *zerolog.Logger) (*Trashbin, error) {
 	return &Trashbin{
 		o:   o,
+		p:   p,
 		lu:  lu,
 		log: log,
 	}, nil
@@ -151,6 +159,18 @@ func (tb *Trashbin) ListRecycle(ctx context.Context, ref *provider.Reference, ke
 		return nil, err
 	}
 
+	// check permissions on the space
+	rp, err := tb.p.AssembleTrashPermissions(ctx, n)
+	switch {
+	case err != nil:
+		return nil, err
+	case !rp.ListRecycle:
+		if rp.Stat {
+			return nil, errtypes.PermissionDenied(key)
+		}
+		return nil, errtypes.NotFound(key)
+	}
+
 	trashRoot := trashRootForNode(n)
 	base := filepath.Join(trashRoot, "files")
 
@@ -232,6 +252,18 @@ func (tb *Trashbin) RestoreRecycleItem(ctx context.Context, ref *provider.Refere
 		return nil, err
 	}
 
+	// check permissions of deleted node
+	rp, err := tb.p.AssembleTrashPermissions(ctx, n)
+	switch {
+	case err != nil:
+		return nil, err
+	case !rp.RestoreRecycleItem:
+		if rp.Stat {
+			return nil, errtypes.PermissionDenied(key)
+		}
+		return nil, errtypes.NotFound(key)
+	}
+
 	trashRoot := trashRootForNode(n)
 	trashPath := filepath.Clean(filepath.Join(trashRoot, "files", key+".trashitem", relativePath))
 
@@ -285,6 +317,18 @@ func (tb *Trashbin) PurgeRecycleItem(ctx context.Context, ref *provider.Referenc
 		return err
 	}
 
+	// check permissions of deleted node
+	rp, err := tb.p.AssembleTrashPermissions(ctx, n)
+	switch {
+	case err != nil:
+		return err
+	case !rp.PurgeRecycle:
+		if rp.Stat {
+			return errtypes.PermissionDenied(key)
+		}
+		return errtypes.NotFound(key)
+	}
+
 	trashRoot := trashRootForNode(n)
 	err = os.RemoveAll(filepath.Clean(filepath.Join(trashRoot, "files", key+".trashitem", relativePath)))
 	if err != nil {
@@ -305,6 +349,18 @@ func (tb *Trashbin) EmptyRecycle(ctx context.Context, ref *provider.Reference) e
 		return err
 	}
 
+	// check permissions of deleted node
+	rp, err := tb.p.AssembleTrashPermissions(ctx, n)
+	switch {
+	case err != nil:
+		return err
+	case !rp.PurgeRecycle:
+		if rp.Stat {
+			return errtypes.PermissionDenied(n.ID)
+		}
+		return errtypes.NotFound(n.ID)
+	}
+
 	trashRoot := trashRootForNode(n)
 	err = os.RemoveAll(filepath.Clean(filepath.Join(trashRoot, "files")))
 	if err != nil {

pkg/storage/fs/posix/trashbin/trashbin_suite_test.go

@@ -0,0 +1,31 @@
+// Copyright 2018-2024 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.
+
+package trashbin_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestTrashbin(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Trashbin Suite")
+}

pkg/storage/fs/posix/trashbin/trashbin_test.go

@@ -0,0 +1,160 @@
+// Copyright 2018-2024 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.
+
+package trashbin_test
+
+import (
+	"github.com/stretchr/testify/mock"
+
+	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	"github.com/owncloud/reva/v2/pkg/errtypes"
+	helpers "github.com/owncloud/reva/v2/pkg/storage/fs/posix/testhelpers"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+// grantTrashPermissions makes the permissions mock answer every
+// AssembleTrashPermissions call with the given permissions.
+func grantTrashPermissions(env *helpers.TestEnv, rp *provider.ResourcePermissions) {
+	env.Permissions.On("AssembleTrashPermissions", mock.Anything, mock.Anything).Return(rp, nil)
+}
+
+var _ = Describe("Trashbin", func() {
+	var (
+		env *helpers.TestEnv
+		ref *provider.Reference
+	)
+
+	BeforeEach(func() {
+		var err error
+		env, err = helpers.NewTestEnv(map[string]interface{}{
+			"metadata_backend": "messagepack",
+			// the permission checks do not involve the fs watcher, and starting
+			// one inotifywait per spec races with the test space setup
+			"watch_fs": false,
+		})
+		Expect(err).ToNot(HaveOccurred())
+
+		// recycle operations address the space, not the deleted item
+		ref = &provider.Reference{ResourceId: env.SpaceRootRes}
+	})
+
+	AfterEach(func() {
+		if env != nil {
+			env.Cleanup()
+		}
+	})
+
+	Context("when the user is not a member of the space", func() {
+		BeforeEach(func() {
+			// AssembleTrashPermissions accumulates grants the user actually holds,
+			// so a non-member ends up with no permissions at all
+			grantTrashPermissions(env, &provider.ResourcePermissions{})
+		})
+
+		It("does not confirm that the space exists when listing", func() {
+			_, err := env.Fs.ListRecycle(env.Ctx, ref, "", "")
+			Expect(err).To(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+
+		It("does not confirm that the space exists when restoring", func() {
+			_, err := env.Fs.RestoreRecycleItem(env.Ctx, ref, "key", "", ref)
+			Expect(err).To(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+
+		It("does not confirm that the space exists when purging", func() {
+			err := env.Fs.PurgeRecycleItem(env.Ctx, ref, "key", "")
+			Expect(err).To(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+
+		It("does not confirm that the space exists when emptying the trash", func() {
+			err := env.Fs.EmptyRecycle(env.Ctx, ref)
+			Expect(err).To(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+	})
+
+	Context("when the user is a viewer", func() {
+		BeforeEach(func() {
+			// a space viewer may browse the trash but not change it,
+			// see conversions.NewSpaceViewerRole()
+			grantTrashPermissions(env, &provider.ResourcePermissions{
+				Stat:        true,
+				ListRecycle: true,
+			})
+		})
+
+		It("allows listing the trash", func() {
+			items, err := env.Fs.ListRecycle(env.Ctx, ref, "", "")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(items).To(BeEmpty())
+		})
+
+		It("denies restoring", func() {
+			_, err := env.Fs.RestoreRecycleItem(env.Ctx, ref, "key", "", ref)
+			Expect(err).To(BeAssignableToTypeOf(errtypes.PermissionDenied("")))
+		})
+
+		It("denies purging a single item", func() {
+			err := env.Fs.PurgeRecycleItem(env.Ctx, ref, "key", "")
+			Expect(err).To(BeAssignableToTypeOf(errtypes.PermissionDenied("")))
+		})
+
+		It("denies emptying the trash", func() {
+			err := env.Fs.EmptyRecycle(env.Ctx, ref)
+			Expect(err).To(BeAssignableToTypeOf(errtypes.PermissionDenied("")))
+		})
+	})
+
+	Context("when the user is an editor", func() {
+		BeforeEach(func() {
+			grantTrashPermissions(env, &provider.ResourcePermissions{
+				Stat:               true,
+				ListRecycle:        true,
+				RestoreRecycleItem: true,
+				PurgeRecycle:       true,
+			})
+		})
+
+		It("allows listing the trash", func() {
+			items, err := env.Fs.ListRecycle(env.Ctx, ref, "", "")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(items).To(BeEmpty())
+		})
+
+		It("allows emptying the trash", func() {
+			Expect(env.Fs.EmptyRecycle(env.Ctx, ref)).To(Succeed())
+		})
+
+		// the item does not exist so these still fail, but they have to fail
+		// on the missing item rather than on the permission check
+		It("lets restoring pass the permission check", func() {
+			_, err := env.Fs.RestoreRecycleItem(env.Ctx, ref, "does-not-exist", "", ref)
+			Expect(err).To(HaveOccurred())
+			Expect(err).ToNot(BeAssignableToTypeOf(errtypes.PermissionDenied("")))
+			Expect(err).ToNot(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+
+		It("lets purging pass the permission check", func() {
+			err := env.Fs.PurgeRecycleItem(env.Ctx, ref, "does-not-exist", "")
+			Expect(err).To(HaveOccurred())
+			Expect(err).ToNot(BeAssignableToTypeOf(errtypes.PermissionDenied("")))
+			Expect(err).ToNot(BeAssignableToTypeOf(errtypes.NotFound("")))
+		})
+	})
+})