Skip to content

Commit

Permalink
allow users with the list-all-spaces permission to list all spaces
Browse files Browse the repository at this point in the history
This implementation is just a quick hack which is definitely not production ready. It should be changed as soon as we have a proper permission system in reva.
  • Loading branch information
David Christofas committed Oct 26, 2021
1 parent fe35bd1 commit d196694
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 13 deletions.
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
13 changes: 12 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,17 @@ 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"]
json.Unmarshal(entry.Value, &permissions)
}

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

0 comments on commit d196694

Please sign in to comment.