Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow users with the list-all-spaces permission to list all spaces #2207

Merged
merged 1 commit into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/list-all-spaces.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Enable users to list all spaces

Added a permission check if the user has the `list-all-spaces` permission.
This enables users to list all spaces, even those which they are not members of.

https://github.com/cs3org/reva/pull/2207
16 changes: 15 additions & 1 deletion internal/grpc/services/storageprovider/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package storageprovider

import (
"context"
"encoding/json"
"fmt"
"net/url"
"os"
Expand Down Expand Up @@ -449,7 +450,20 @@ func hasNodeID(s *provider.StorageSpace) bool {

func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSpacesRequest) (*provider.ListStorageSpacesResponse, error) {
log := appctx.GetLogger(ctx)
spaces, err := s.storage.ListStorageSpaces(ctx, req.Filters)

// This is just a quick hack to get the users permission into reva.
// Replace this as soon as we have a proper system to check the users permissions.
opaque := req.Opaque
var permissions map[string]struct{}
if opaque != nil {
entry := opaque.Map["permissions"]
err := json.Unmarshal(entry.Value, &permissions)
if err != nil {
return nil, err
}
}

spaces, err := s.storage.ListStorageSpaces(ctx, req.Filters, permissions)
if err != nil {
var st *rpc.Status
switch err.(type) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/nextcloud/nextcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ func (nc *StorageDriver) UnsetArbitraryMetadata(ctx context.Context, ref *provid
}

// 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, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
bodyStr, _ := json.Marshal(f)
_, respBody, err := nc.do(ctx, Action{"ListStorageSpaces", string(bodyStr)})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/nextcloud/nextcloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,7 @@ var _ = Describe("Nextcloud", func() {
},
}
filters := []*provider.ListStorageSpacesRequest_Filter{filter1, filter2, filter3}
spaces, err := nc.ListStorageSpaces(ctx, filters)
spaces, err := nc.ListStorageSpaces(ctx, filters, nil)
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
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/owncloud/owncloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -2230,7 +2230,7 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, basePath, key, relativeP
return fs.propagate(ctx, tgt)
}

func (fs *ocfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error) {
func (fs *ocfs) ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
return nil, errtypes.NotSupported("list storage spaces")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/owncloudsql/owncloudsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,7 @@ func (fs *owncloudsqlfs) HashFile(path string) (string, string, string, error) {
}
}

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, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
// TODO(corby): Implement
return nil, errtypes.NotSupported("list storage spaces")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/s3/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ func (fs *s3FS) RestoreRecycleItem(ctx context.Context, basePath, key, relativeP
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, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
return nil, errtypes.NotSupported("list storage spaces")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type FS interface {
Shutdown(ctx context.Context) error
SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) error
UnsetArbitraryMetadata(ctx context.Context, ref *provider.Reference, keys []string) error
ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter) ([]*provider.StorageSpace, error)
ListStorageSpaces(ctx context.Context, filter []*provider.ListStorageSpacesRequest_Filter, permissions map[string]struct{}) ([]*provider.StorageSpace, error)
CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error)
UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/storage/utils/decomposedfs/spaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
// 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, permissions map[string]struct{}) ([]*provider.StorageSpace, error) {
// TODO check filters

// TODO when a space symlink is broken delete the space for cleanup
Expand Down Expand Up @@ -226,7 +226,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
}

// TODO apply more filters
space, err := fs.storageSpaceFromNode(ctx, n, matches[i], spaceType)
space, err := fs.storageSpaceFromNode(ctx, n, matches[i], spaceType, permissions)
if err != nil {
appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space")
continue
Expand Down Expand Up @@ -318,7 +318,7 @@ func (fs *Decomposedfs) createStorageSpace(ctx context.Context, spaceType, nodeI
return nil
}

func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, node *node.Node, nodePath, spaceType string) (*provider.StorageSpace, error) {
func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, node *node.Node, nodePath, spaceType string, permissions map[string]struct{}) (*provider.StorageSpace, error) {
owner, err := node.Owner()
if err != nil {
return nil, err
Expand Down Expand Up @@ -346,11 +346,12 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, node *node.Nod
user := ctxpkg.ContextMustGetUser(ctx)

// filter out spaces user cannot access (currently based on stat permission)
_, canListAllSpaces := permissions["list-all-spaces"]
p, err := node.ReadUserPermissions(ctx, user)
if err != nil {
return nil, err
}
if !p.Stat {
if !(canListAllSpaces || p.Stat) {
return nil, errors.New("user is not allowed to Stat the space")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/utils/eosfs/eosfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1551,7 +1551,7 @@ func (fs *eosfs) RestoreRecycleItem(ctx context.Context, basePath, key, relative
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, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
return nil, errtypes.NotSupported("list storage spaces")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/utils/localfs/localfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@ func (fs *localfs) RestoreRecycleItem(ctx context.Context, basePath, key, relati
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, _ map[string]struct{}) ([]*provider.StorageSpace, error) {
return nil, errtypes.NotSupported("list storage spaces")
}

Expand Down