add test struct

Signed-off-by: Christian Richter <crichter@owncloud.com>
Co-authored-by: André Duffeck <andre.duffeck@firondu.de>

pkg/storage/storage.go

@@ -74,7 +74,7 @@ type FS interface {
 	UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error)
 	DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) error
 	CanListSpacesOfRequestedUser(ctx context.Context, requestedUserID string) bool
-	CheckNodePermissions(ctx context.Context, requestedUserID string, unrestricted bool) bool
+	MustCheckNodePermissions(ctx context.Context, requestedUserID string, unrestricted bool) bool
 }
 
 // Registry is the interface that storage registries implement

pkg/storage/utils/decomposedfs/spaces.go

@@ -283,7 +283,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 		return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list spaces of other users", authenticatedUserID))
 	}
 
-	checkNodePermissions := fs.CheckNodePermissions(ctx, requestedUserID, unrestricted)
+	checkNodePermissions := fs.MustCheckNodePermissions(ctx, requestedUserID, unrestricted)
 
 	spaces := []*provider.StorageSpace{}
 	// build the glob path, eg.
@@ -428,8 +428,8 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 
 }
 
-// Checks if permission checks are need to be performed when user requests spaces
-func (fs *Decomposedfs) CheckNodePermissions(ctx context.Context, requestedUserID string, unrestricted bool) bool {
+// MustCheckNodePermissions checks if permission checks are needed to be performed when user requests spaces
+func (fs *Decomposedfs) MustCheckNodePermissions(ctx context.Context, requestedUserID string, unrestricted bool) bool {
 	authenticatedUserID := ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId()
 	canListAllSpaces := fs.canListAllSpaces(ctx)
 	switch {
@@ -439,13 +439,12 @@ func (fs *Decomposedfs) CheckNodePermissions(ctx context.Context, requestedUserI
 		// as admin you have to be able to see other users spaces
 		return false
 	case canListAllSpaces, unrestricted:
-		// standard case, requesting user needs to check space permissions
 		return false
 	}
 	return true
 }
 
-// Checks if user is allowed to list spaces of another user
+// CanListSpacesOfRequestedUser checks if user is allowed to list spaces of another user
 func (fs *Decomposedfs) CanListSpacesOfRequestedUser(ctx context.Context, requestedUserID string) bool {
 	authenticatedUserID := ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId()
 	switch {

pkg/storage/utils/decomposedfs/spaces_test.go

@@ -19,16 +19,21 @@
 package decomposedfs_test
 
 import (
+	"context"
 	"os"
 
+	cs3permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1"
 	permissionsv1beta1 "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1"
 	rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
+	ruser "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs"
 	helpers "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/testhelpers"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/mock"
+	"google.golang.org/grpc"
 )
 
 var _ = Describe("Spaces", func() {
@@ -41,7 +46,17 @@ var _ = Describe("Spaces", func() {
 			var err error
 			env, err = helpers.NewTestEnv(nil)
 			Expect(err).ToNot(HaveOccurred())
-			env.PermissionsClient.On("CheckPermission", mock.Anything, mock.Anything, mock.Anything).Return(&permissionsv1beta1.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}}, nil)
+			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
+						return &permissionsv1beta1.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}}
+					}
+					// id of generic user
+					return &permissionsv1beta1.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_PERMISSION_DENIED}}
+				},
+				nil)
+
 		})
 
 		AfterEach(func() {
@@ -71,6 +86,44 @@ var _ = Describe("Spaces", func() {
 				Expect(resp.StorageSpace.SpaceType).To(Equal("project"))
 			})
 		})
+
+		Context("needs to check node permissions", func() {
+			It("returns true on requesting for other user as non-admin", func() {
+				ctx := ruser.ContextSetUser(context.Background(), env.Users[0])
+				resp := env.Fs.MustCheckNodePermissions(ctx, helpers.User0ID, false)
+				Expect(resp).To(Equal(true))
+			})
+			It("returns false on requesting for other user as admin", func() {
+				resp := env.Fs.MustCheckNodePermissions(env.Ctx, helpers.User0ID, false)
+				Expect(resp).To(Equal(false))
+			})
+			It("returns true on requesting for own spaces", func() {
+				ctx := ruser.ContextSetUser(context.Background(), env.Users[0])
+				resp := env.Fs.MustCheckNodePermissions(ctx, helpers.User0ID, false)
+				Expect(resp).To(Equal(true))
+			})
+			It("returns false on unrestricted", func() {
+				resp := env.Fs.MustCheckNodePermissions(env.Ctx, "some-uuid-that-does-not-make-sense", true)
+				Expect(resp).To(Equal(false))
+			})
+		})
+
+		Context("can list spaces of requested user", func() {
+			It("returns false on requesting for other user as non-admin", func() {
+				ctx := ruser.ContextSetUser(context.Background(), env.Users[0])
+				res := env.Fs.CanListSpacesOfRequestedUser(ctx, helpers.User1ID)
+				Expect(res).To(Equal(false))
+			})
+			It("returns true on requesting for other user as admin", func() {
+				res := env.Fs.CanListSpacesOfRequestedUser(env.Ctx, helpers.User0ID)
+				Expect(res).To(Equal(true))
+			})
+			It("returns true on requesting for own spaces", func() {
+				res := env.Fs.CanListSpacesOfRequestedUser(env.Ctx, helpers.OwnerID)
+				Expect(res).To(Equal(true))
+			})
+		})
+
 		Describe("Create Spaces with custom alias template", func() {
 			var (
 				env *helpers.TestEnv

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

@@ -53,12 +53,19 @@ type TestEnv struct {
 	Permissions       *mocks.PermissionsChecker
 	Blobstore         *treemocks.Blobstore
 	Owner             *userpb.User
+	Users             []*userpb.User
 	Lookup            *lookup.Lookup
 	Ctx               context.Context
 	SpaceRootRes      *providerv1beta1.ResourceId
 	PermissionsClient *mocks.CS3PermissionsClient
 }
 
+const (
+	OwnerID = "25b69780-5f39-43be-a7ac-a9b9e9fe4230"
+	User0ID = "824385ae-8fc6-4896-8eb2-d1d171290bd0"
+	User1ID = "693b0d96-80a2-4016-b53d-425ce4f66114"
+)
+
 // NewTestEnv prepares a test environment on disk
 // The storage contains some directories and a file:
 //
@@ -93,11 +100,27 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 	owner := &userpb.User{
 		Id: &userpb.UserId{
 			Idp:      "idp",
-			OpaqueId: "25b69780-5f39-43be-a7ac-a9b9e9fe4230",
+			OpaqueId: OwnerID,
 			Type:     userpb.UserType_USER_TYPE_PRIMARY,
 		},
 		Username: "username",
 	}
+	users := []*userpb.User{
+		{
+			Id: &userpb.UserId{
+				Idp:      "idp",
+				OpaqueId: User0ID,
+				Type:     userpb.UserType_USER_TYPE_PRIMARY,
+			},
+		},
+		{
+			Id: &userpb.UserId{
+				Idp:      "idp",
+				OpaqueId: User1ID,
+				Type:     userpb.UserType_USER_TYPE_PRIMARY,
+			},
+		},
+	}
 	lookup := &lookup.Lookup{Options: o}
 	permissions := &mocks.PermissionsChecker{}
 	cs3permissionsclient := &mocks.CS3PermissionsClient{}
@@ -117,6 +140,7 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 		Permissions:       permissions,
 		Blobstore:         bs,
 		Owner:             owner,
+		Users:             users,
 		Ctx:               ctx,
 		PermissionsClient: cs3permissionsclient,
 	}