add flag to unrestrict spaces listing (#2799)

cs3org · Apr 29, 2022 · 71d0c17 · 71d0c17
1 parent 599bdac
commit 71d0c17
Show file tree

Hide file tree

Showing 13 changed files with 55 additions and 29 deletions.
diff --git a/changelog/unreleased/list-spaces-unrestricted.md b/changelog/unreleased/list-spaces-unrestricted.md
@@ -0,0 +1,6 @@
+Enhancement: Add flag to enable unrestriced listing of spaces
+
+Listing spaces now only returns all spaces when the user has the permissions and it was explicitly requested.
+The default will only return the spaces the current user has access to.
+
+https://github.com/cs3org/reva/pull/2799
diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go
@@ -520,7 +520,15 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 
 	log := appctx.GetLogger(ctx)
 
-	spaces, err := s.storage.ListStorageSpaces(ctx, req.Filters)
+	// TODO this is just temporary. Update the API to include this flag.
+	unrestricted := false
+	if req.Opaque != nil {
+		if entry, ok := req.Opaque.Map["unrestricted"]; ok {
+			unrestricted, _ = strconv.ParseBool(string(entry.Value))
+		}
+	}
+
+	spaces, err := s.storage.ListStorageSpaces(ctx, req.Filters, unrestricted)
 	if err != nil {
 		var st *rpc.Status
 		switch err.(type) {

diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go
@@ -795,7 +795,7 @@ func (nc *StorageDriver) Unlock(ctx context.Context, ref *provider.Reference, lo
 }
 
 // ListStorageSpaces as defined in the storage.FS interface
-func (nc *StorageDriver) ListStorageSpaces(ctx context.Context, f []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (nc *StorageDriver) ListStorageSpaces(ctx context.Context, f []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	bodyStr, _ := json.Marshal(f)
 	_, respBody, err := nc.do(ctx, Action{"ListStorageSpaces", string(bodyStr)})
 	if err != nil {

diff --git a/pkg/storage/fs/nextcloud/nextcloud_test.go b/pkg/storage/fs/nextcloud/nextcloud_test.go
@@ -982,7 +982,7 @@ var _ = Describe("Nextcloud", func() {
 				},
 			}
 			filters := []*provider.ListStorageSpacesRequest_Filter{filter1, filter2, filter3}
-			spaces, err := nc.ListStorageSpaces(ctx, filters)
+			spaces, err := nc.ListStorageSpaces(ctx, filters, false)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(spaces)).To(Equal(1))
 			// https://github.com/cs3org/go-cs3apis/blob/970eec3/cs3/storage/provider/v1beta1/resources.pb.go#L1341-L1366

diff --git a/pkg/storage/fs/owncloudsql/spaces.go b/pkg/storage/fs/owncloudsql/spaces.go
@@ -34,7 +34,7 @@ import (
 )
 
 // ListStorageSpaces lists storage spaces according to the provided filters
-func (fs *owncloudsqlfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (fs *owncloudsqlfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	var (
 		spaceID = "*"
 	)

diff --git a/pkg/storage/fs/s3/s3.go b/pkg/storage/fs/s3/s3.go
@@ -702,7 +702,7 @@ func (fs *s3FS) RestoreRecycleItem(ctx context.Context, ref *provider.Reference,
 	return errtypes.NotSupported("restore recycle")
 }
 
-func (fs *s3FS) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (fs *s3FS) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	return nil, errtypes.NotSupported("list storage spaces")
 }
 

diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go
@@ -267,18 +267,19 @@ func (r *registry) GetProvider(ctx context.Context, space *providerpb.StorageSpa
 // below   = /foo/bar/bif   <=> /foo/bar        -> list(spaceid, ./bif)
 func (r *registry) ListProviders(ctx context.Context, filters map[string]string) ([]*registrypb.ProviderInfo, error) {
 	b, _ := strconv.ParseBool(filters["unique"])
+	unrestricted, _ := strconv.ParseBool(filters["unrestricted"])
 	switch {
 	case filters["storage_id"] != "" && filters["opaque_id"] != "":
 		findMountpoint := filters["type"] == "mountpoint"
 		findGrant := !findMountpoint && filters["path"] == "" // relvative references, by definition, occur in the correct storage, so do not look for grants
-		return r.findProvidersForResource(ctx, filters["storage_id"]+"!"+filters["opaque_id"], findMountpoint, findGrant), nil
+		return r.findProvidersForResource(ctx, filters["storage_id"]+"!"+filters["opaque_id"], findMountpoint, findGrant, unrestricted), nil
 	case filters["path"] != "":
-		return r.findProvidersForAbsolutePathReference(ctx, filters["path"], b), nil
+		return r.findProvidersForAbsolutePathReference(ctx, filters["path"], b, unrestricted), nil
 	case len(filters) == 0:
 		// return all providers
 		return r.findAllProviders(ctx), nil
 	default:
-		return r.findProvidersForFilter(ctx, r.buildFilters(filters)), nil
+		return r.findProvidersForFilter(ctx, r.buildFilters(filters), unrestricted), nil
 	}
 }
 
@@ -318,15 +319,15 @@ func (r *registry) buildFilters(filterMap map[string]string) []*providerpb.ListS
 	return filters
 }
 
-func (r *registry) findProvidersForFilter(ctx context.Context, filters []*providerpb.ListStorageSpacesRequest_Filter) []*registrypb.ProviderInfo {
+func (r *registry) findProvidersForFilter(ctx context.Context, filters []*providerpb.ListStorageSpacesRequest_Filter, unrestricted bool) []*registrypb.ProviderInfo {
 
 	currentUser := ctxpkg.ContextMustGetUser(ctx)
 	providerInfos := []*registrypb.ProviderInfo{}
 	for address, provider := range r.c.Providers {
 		p := &registrypb.ProviderInfo{
 			Address: address,
 		}
-		spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters)
+		spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters, unrestricted)
 		if err != nil {
 			appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider by id failed, continuing")
 			continue
@@ -364,7 +365,7 @@ func (r *registry) findProvidersForFilter(ctx context.Context, filters []*provid
 // findProvidersForResource looks up storage providers based on a resource id
 // for the root of a space the res.StorageId is the same as the res.OpaqueId
 // for share spaces the res.StorageId tells the registry the spaceid and res.OpaqueId is a node in that space
-func (r *registry) findProvidersForResource(ctx context.Context, id string, findMoundpoint, findGrant bool) []*registrypb.ProviderInfo {
+func (r *registry) findProvidersForResource(ctx context.Context, id string, findMoundpoint, findGrant, unrestricted bool) []*registrypb.ProviderInfo {
 	currentUser := ctxpkg.ContextMustGetUser(ctx)
 	providerInfos := []*registrypb.ProviderInfo{}
 	for address, provider := range r.c.Providers {
@@ -398,7 +399,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string, find
 				},
 			})
 		}
-		spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters)
+		spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters, false)
 		if err != nil {
 			appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider by id failed, continuing")
 			continue
@@ -451,7 +452,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string, find
 
 // findProvidersForAbsolutePathReference takes a path and returns the storage provider with the longest matching path prefix
 // FIXME use regex to return the correct provider when multiple are configured
-func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, path string, unique bool) []*registrypb.ProviderInfo {
+func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, path string, unique, unrestricted bool) []*registrypb.ProviderInfo {
 	currentUser := ctxpkg.ContextMustGetUser(ctx)
 
 	deepestMountPath := ""
@@ -474,7 +475,7 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa
 			},
 		})
 
-		spaces, err = r.findStorageSpaceOnProvider(ctx, p.Address, filters)
+		spaces, err = r.findStorageSpaceOnProvider(ctx, p.Address, filters, unrestricted)
 		if err != nil {
 			appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider failed, continuing")
 			continue
@@ -598,12 +599,20 @@ func setSpaces(providerInfo *registrypb.ProviderInfo, spaces []*providerpb.Stora
 	return nil
 }
 
-func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string, filters []*providerpb.ListStorageSpacesRequest_Filter) ([]*providerpb.StorageSpace, error) {
+func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string, filters []*providerpb.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*providerpb.StorageSpace, error) {
 	c, err := r.getStorageProviderServiceClient(addr)
 	if err != nil {
 		return nil, err
 	}
 	req := &providerpb.ListStorageSpacesRequest{
+		Opaque: &typesv1beta1.Opaque{
+			Map: map[string]*typesv1beta1.OpaqueEntry{
+				"unrestricted": {
+					Decoder: "plain",
+					Value:   []byte(strconv.FormatBool(unrestricted)),
+				},
+			},
+		},
 		Filters: filters,
 	}
 

diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
@@ -62,7 +62,10 @@ type FS interface {
 	GetLock(ctx context.Context, ref *provider.Reference) (*provider.Lock, error)
 	RefreshLock(ctx context.Context, ref *provider.Reference, lock *provider.Lock) error
 	Unlock(ctx context.Context, ref *provider.Reference, lock *provider.Lock) error
-	ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error)
+	// ListStorageSpaces lists the spaces in the storage.
+	// The unrestricted parameter can be used to list other user's spaces when
+	// the user has the necessary permissions.
+	ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error)
 	CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error)
 	UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error)
 	DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) error

diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go
@@ -160,7 +160,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
 		}
 	}
 
-	space, err := fs.storageSpaceFromNode(ctx, root, root.InternalPath(), false)
+	space, err := fs.storageSpaceFromNode(ctx, root, root.InternalPath(), false, false)
 	if err != nil {
 		return nil, err
 	}
@@ -235,7 +235,7 @@ func readSpaceAndNodeFromSpaceTypeLink(path string) (string, string, error) {
 // The list can be filtered by space type or space id.
 // Spaces are persisted with symlinks in /spaces/<type>/<spaceid> pointing to ../../nodes/<nodeid>, the root node of the space
 // The spaceid is a concatenation of storageid + "!" + nodeid
-func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	// TODO check filters
 
 	// TODO when a space symlink is broken delete the space for cleanup
@@ -294,7 +294,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 			// return empty list
 			return spaces, nil
 		}
-		space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces)
+		space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces, unrestricted)
 		if err != nil {
 			return nil, err
 		}
@@ -364,7 +364,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 		}
 
 		// TODO apply more filters
-		space, err := fs.storageSpaceFromNode(ctx, n, matches[i], canListAllSpaces)
+		space, err := fs.storageSpaceFromNode(ctx, n, matches[i], canListAllSpaces, unrestricted)
 		if err != nil {
 			if _, ok := err.(errtypes.IsPermissionDenied); !ok {
 				appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space")
@@ -382,7 +382,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 			return nil, err
 		}
 		if n.Exists {
-			space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces)
+			space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), canListAllSpaces, unrestricted)
 			if err != nil {
 				return nil, err
 			}
@@ -484,7 +484,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up
 	}
 
 	// send back the updated data from the storage
-	updatedSpace, err := fs.storageSpaceFromNode(ctx, node, node.InternalPath(), false)
+	updatedSpace, err := fs.storageSpaceFromNode(ctx, node, node.InternalPath(), false, false)
 	if err != nil {
 		return nil, err
 	}
@@ -562,9 +562,9 @@ func (fs *Decomposedfs) linkStorageSpaceType(ctx context.Context, spaceType stri
 	return err
 }
 
-func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, nodePath string, canListAllSpaces bool) (*provider.StorageSpace, error) {
+func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, nodePath string, canListAllSpaces bool, unrestricted bool) (*provider.StorageSpace, error) {
 	user := ctxpkg.ContextMustGetUser(ctx)
-	if !canListAllSpaces {
+	if !canListAllSpaces || !unrestricted {
 		ok, err := node.NewPermissions(fs.lu).HasPermission(ctx, n, func(p *provider.ResourcePermissions) bool {
 			return p.Stat
 		})

diff --git a/pkg/storage/utils/decomposedfs/spaces_test.go b/pkg/storage/utils/decomposedfs/spaces_test.go
@@ -48,7 +48,7 @@ var _ = Describe("Create Spaces", func() {
 
 	Context("during login", func() {
 		It("space is created", func() {
-			resp, err := env.Fs.ListStorageSpaces(env.Ctx, nil)
+			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"))
@@ -89,7 +89,7 @@ var _ = Describe("Create Spaces", func() {
 		})
 		Context("during login", func() {
 			It("personal space is created with custom alias", func() {
-				resp, err := env.Fs.ListStorageSpaces(env.Ctx, nil)
+				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@_unknown"))

diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go
@@ -1603,7 +1603,7 @@ func (fs *eosfs) RestoreRecycleItem(ctx context.Context, ref *provider.Reference
 	return fs.c.RestoreDeletedEntry(ctx, auth, key)
 }
 
-func (fs *eosfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (fs *eosfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	return nil, errtypes.NotSupported("list storage spaces")
 }
 

diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go
@@ -1290,7 +1290,7 @@ func (fs *localfs) RestoreRecycleItem(ctx context.Context, ref *provider.Referen
 	return fs.propagate(ctx, localRestorePath)
 }
 
-func (fs *localfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
+func (fs *localfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, unrestricted bool) ([]*provider.StorageSpace, error) {
 	return nil, errtypes.NotSupported("list storage spaces")
 }
 

diff --git a/tests/integration/grpc/gateway_storageprovider_test.go b/tests/integration/grpc/gateway_storageprovider_test.go
@@ -373,7 +373,7 @@ var _ = Describe("gateway", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(r.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
-			spaces, err := fs.ListStorageSpaces(ctx, []*storagep.ListStorageSpacesRequest_Filter{})
+			spaces, err := fs.ListStorageSpaces(ctx, []*storagep.ListStorageSpacesRequest_Filter{}, false)
 			Expect(err).ToNot(HaveOccurred())
 			homeSpace = spaces[0]