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.

Sign up for GitHub

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

Jump to bottom

[tests-only] Spaces registry PR comments #2383

Merged
merged 2 commits into from
Dec 23, 2021
Merged

[tests-only] Spaces registry PR comments #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf4645f
fix valid points mentioned on review
kobergj Dec 17, 2021
2e36d97
fix linting
kobergj Dec 17, 2021
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
21 changes: 0 additions & 21 deletions internal/grpc/interceptors/auth/scope.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,27 +209,6 @@ func checkIfNestedResource(ctx context.Context, ref *provider.Reference, parent
}

return strings.HasPrefix(childPath, parentPath), nil

// resourcePath := statResponse.Info.Path

// if strings.HasPrefix(ref.GetPath(), resourcePath) {
// // The path corresponds to the resource to which the token has access.
// // We allow access to it.
// return true, nil
// }

// // If we arrived here that could mean that ref.GetPath is not prefixed with the storage mount path but resourcePath is
// // because it was returned by the gateway which will prefix it. To fix that we remove the mount path from the resourcePath.
// // resourcePath = "/users/<name>/some/path"
// // After the split we have [" ", "users", "<name>/some/path"].
// trimmedPath := "/" + strings.SplitN(resourcePath, "/", 3)[2]
// if strings.HasPrefix(ref.GetPath(), trimmedPath) {
// // The path corresponds to the resource to which the token has access.
// // We allow access to it.
// return true, nil
// }

// return false, nil
}

func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) {
Expand Down
4 changes: 1 addition & 3 deletions internal/grpc/services/gateway/appprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ import (
)

func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*providerpb.OpenInAppResponse, error) {

resChild := ""
statRes, err := s.Stat(ctx, &storageprovider.StatRequest{
Ref: req.Ref,
})
Expand All @@ -72,7 +70,7 @@ func (s *svc) OpenInApp(ctx context.Context, req *gateway.OpenInAppRequest) (*pr
}
if uri.Scheme == "webdav" {
insecure, skipVerify := getGRPCConfig(req.Opaque)
return s.openFederatedShares(ctx, fileInfo.Target, req.ViewMode, req.App, insecure, skipVerify, resChild)
return s.openFederatedShares(ctx, fileInfo.Target, req.ViewMode, req.App, insecure, skipVerify, "")
}

res, err := s.Stat(ctx, &storageprovider.StatRequest{
Expand Down
20 changes: 10 additions & 10 deletions internal/grpc/services/gateway/ocmshareprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,16 +359,16 @@ func (s *svc) createOCMReference(ctx context.Context, share *ocm.Share) (*rpc.St
return status.NewInternal(ctx, "error finding storage provider"), nil
}

spaceID := ""
mountPath := p.ProviderPath
var root *provider.ResourceId

spacePaths := decodeSpacePaths(p.Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
var (
root *provider.ResourceId
mountPath string
)
for spaceID, mp := range decodeSpacePaths(p) {
rootSpace, rootNode, err := utils.SplitStorageSpaceID(spaceID)
if err != nil {
continue
}
mountPath = mp
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
Expand Down
199 changes: 58 additions & 141 deletions internal/grpc/services/gateway/storageprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package gateway
import (
"context"
"encoding/json"
"encoding/xml"
"fmt"
"net/url"
"path"
Expand All @@ -29,6 +30,7 @@ import (
"sync"
"time"

"github.com/BurntSushi/toml"
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
Expand Down Expand Up @@ -217,7 +219,11 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp
for _, f := range req.Filters {
switch f.Type {
case provider.ListStorageSpacesRequest_Filter_TYPE_ID:
filters["storage_id"], filters["opaque_id"] = utils.SplitStorageSpaceID(f.GetId().OpaqueId)
sid, oid, err := utils.SplitStorageSpaceID(f.GetId().OpaqueId)
if err != nil {
continue
}
filters["storage_id"], filters["opaque_id"] = sid, oid
case provider.ListStorageSpacesRequest_Filter_TYPE_OWNER:
filters["owner_idp"] = f.GetOwner().Idp
filters["owner_id"] = f.GetOwner().OpaqueId
Expand Down Expand Up @@ -338,7 +344,13 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag
func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*provider.DeleteStorageSpaceResponse, error) {
log := appctx.GetLogger(ctx)
// TODO: needs to be fixed
storageid, opaqeid := utils.SplitStorageSpaceID(req.Id.OpaqueId)
storageid, opaqeid, err := utils.SplitStorageSpaceID(req.Id.OpaqueId)
if err != nil {
return &provider.DeleteStorageSpaceResponse{
Status: status.NewInvalidArg(ctx, "OpaqueID was empty"),
}, nil
}

c, _, err := s.find(ctx, &provider.Reference{ResourceId: &provider.ResourceId{
StorageId: storageid,
OpaqueId: opaqeid,
Expand Down Expand Up @@ -404,10 +416,8 @@ func (s *svc) GetHome(ctx context.Context, _ *provider.GetHomeRequest) (*provide
}, nil
}

spacePaths := decodeSpacePaths(res.Providers[0].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = res.Providers[0].ProviderPath
}
// NOTE: this will cause confusion if len(spacePath) > 1
spacePaths := decodeSpacePaths(res.Providers[0])
for _, spacePath := range spacePaths {
return &provider.GetHomeResponse{
Path: spacePath,
Expand Down Expand Up @@ -725,16 +735,9 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St
continue
}

spaceID := ""
mountPath := providerInfos[i].ProviderPath

spacePaths := decodeSpacePaths(providerInfos[i].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
for spaceID, mountPath := range decodeSpacePaths(providerInfos[i]) {
var root *provider.ResourceId
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID)
if rootSpace != "" && rootNode != "" {
root = &provider.ResourceId{
StorageId: rootSpace,
Expand Down Expand Up @@ -890,16 +893,9 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ
continue
}

spaceID := ""
mountPath := providerInfos[i].ProviderPath

spacePaths := decodeSpacePaths(providerInfos[i].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
for spaceID, mountPath := range decodeSpacePaths(providerInfos[i]) {
var root *provider.ResourceId
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID)
if rootSpace != "" && rootNode != "" {
root = &provider.ResourceId{
StorageId: rootSpace,
Expand Down Expand Up @@ -1104,16 +1100,9 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest)
}, nil
}

spaceID := ""
mountPath := providerInfos[i].ProviderPath
var root *provider.ResourceId

spacePaths := decodeSpacePaths(providerInfos[i].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
for spaceID, mountPath := range decodeSpacePaths(providerInfos[i]) {
rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID)
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
Expand Down Expand Up @@ -1183,16 +1172,9 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc
var srcRef *provider.Reference
for i := range providerInfos {

spaceID := ""
mountPath := providerInfos[i].ProviderPath
var root *provider.ResourceId

spacePaths := decodeSpacePaths(providerInfos[i].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
for spaceID, mountPath := range decodeSpacePaths(providerInfos[i]) {
rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID)
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
Expand Down Expand Up @@ -1232,16 +1214,9 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc
var dstProvider *registry.ProviderInfo
var dstRef *provider.Reference
for i := range dstProviderInfos {
spaceID := ""
mountPath := dstProviderInfos[i].ProviderPath
var root *provider.ResourceId

spacePaths := decodeSpacePaths(dstProviderInfos[i].Opaque)
if len(spacePaths) == 0 {
spacePaths[""] = mountPath
}
for spaceID, mountPath = range spacePaths {
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
for spaceID, mountPath := range decodeSpacePaths(dstProviderInfos[i]) {
rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID)
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
Expand Down Expand Up @@ -1395,20 +1370,23 @@ func (s *svc) findAndUnwrap(ctx context.Context, ref *provider.Reference) (provi
return nil, nil, nil, err
}

mountPath := p.ProviderPath
var root *provider.ResourceId

if spacePaths := decodeSpacePaths(p.Opaque); len(spacePaths) > 0 {
for spaceID, spacePath := range spacePaths {
mountPath = spacePath
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
}
break // TODO can there be more than one space for a path?
var (
root *provider.ResourceId
mountPath string
)
for spaceID, spacePath := range decodeSpacePaths(p) {
mountPath = spacePath
rootSpace, rootNode, err := utils.SplitStorageSpaceID(spaceID)
if err != nil {
continue
}
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
}
break // TODO can there be more than one space for a path?
}

relativeReference := unwrap(ref, mountPath, root)

return c, p, relativeReference, nil
Expand Down Expand Up @@ -1529,77 +1507,7 @@ func (s *svc) findProviders(ctx context.Context, ref *provider.Reference) ([]*re
if err = s.providerCache.Set(ref.ResourceId.StorageId, res.Providers); err != nil {
appctx.GetLogger(ctx).Warn().Err(err).Interface("reference", ref).Msg("gateway: could not cache providers")
}
} /* else {
// every user has a cache for mount points?
// the path map must be cached in the registry, not in the gateway?
// - in the registry we cannot determine if other spaces have been mounted or removed. if a new project space was mounted that happens in the registry
// - but the registry does not know when we rename a space ... or does it?
// - /.../Shares is a collection the gateway builds by aggregating the liststoragespaces response
// - the spaces registry builds a path for every space, treating every share as a distinct space.
// - findProviders() will return a long list of spaces, the Stat / ListContainer calls will stat the root etags of every space and share
// -> FIXME cache the root etag of every space, ttl ... do we need to stat? or can we cach the root etag in the providerinfo?
// - large amounts of shares
// use the root etag of a space to determine if we can read from cache?
// (finished) uploads, created dirs, renamed nodes, deleted nodes cause the root etag of a space to change
//
var providersCache *ttlcache.Cache
cache, err := s.mountCache.Get(userKey(ctx))
if err != nil {
providersCache = ttlcache.NewCache()
_ = providersCache.SetTTL(time.Duration(s.c.MountCacheTTL) * time.Second)
providersCache.SkipTTLExtensionOnHit(true)
s.mountCache.Set(userKey(ctx), providersCache)
} else {
providersCache = cache.(*ttlcache.Cache)
}

for _, providerInfo := range res.Providers {

mountPath := providerInfo.ProviderPath
var root *provider.ResourceId

if spacePaths := decodeSpacePaths(p.Opaque); len(spacePaths) > 0 {
for spaceID, spacePath := range spacePaths {
mountPath = spacePath
rootSpace, rootNode := utils.SplitStorageSpaceID(spaceID)
root = &provider.ResourceId{
StorageId: rootSpace,
OpaqueId: rootNode,
}
break // TODO can there be more than one space for a path?
}
}
providersCache.Set(userKey(ctx), res.Providers) // FIXME needs a map[string]*registry.ProviderInfo

}
// use ListProviders? make it return all providers a user has access to aka all mount points?
// cache that list in the gateway.
// -> invalidate the cached list of mountpoints when a modification happens
// refres by loading all mountpoints from spaces registry
// - in the registry cache listStorageSpaces responses for every provider so we don't have to query every provider?
// - how can we determine which listStorageSpaces response to invalidate?
// - misuse ListContainerStream to get notified of root changes of every space?
// - or send a ListStorageSpaces request to the registry with an invalidate(spaceid) property?
// - This would allow the gateway could tell the registry which space(s) to refresh
// - but the registry might not be using a cache
// - we still don't know when an upload finishes ... so we cannot invalidate the cache for that event
// - especially if there are workflows involved?
// - actually, the initiate upload response should make the provider show the file immediately. it should not be downloadable though
// - with stat we want to see the progress. actually multiple uploads (-> workflows) to the same file might be in progress...
// example:
// - user accepts a share in the web ui, then navigates into his /Shares folder
// -> he should see the accepted share, and he should be able to navigate into it
// - actually creating a share should already create a space, but it has no name yet
// - the problem arises when someone mounts a spaece (can pe a share or a project, does not matter)
// -> when do we update the list of mount points which we cache in the gateway?
// - we want to maintain a list of all mount points (and their root etag/mtime) to allow clients to efficiently poll /
// and query the list of all storage spaces the user has access to
// - the simplest 'maintenance' is caching the complete list and invalidating it on changes
// - a more elegant 'maintenance' would add and remove paths as they occur ... which is what the spaces registry is supposed to do...
// -> don't cache anything in the gateway for path based requests. Instead maintain a cache in the spaces registry.
//
// Caching needs to take the last modification time into account to discover new mount points -> needs to happen in the registry
}*/
}

return res.Providers, nil
}
Expand All @@ -1617,7 +1525,7 @@ func unwrap(ref *provider.Reference, mountPoint string, root *provider.ResourceI
}
return providerRef
}
// build a copy to avoid side effects

return &provider.Reference{
ResourceId: &provider.ResourceId{
StorageId: ref.ResourceId.StorageId,
Expand All @@ -1627,14 +1535,23 @@ func unwrap(ref *provider.Reference, mountPoint string, root *provider.ResourceI
}
}

func decodeSpacePaths(o *typesv1beta1.Opaque) map[string]string {
func decodeSpacePaths(r *registry.ProviderInfo) map[string]string {
spacePaths := map[string]string{}
if o == nil {
return spacePaths
if r.Opaque != nil {
if entry, ok := r.Opaque.Map["space_paths"]; ok {
switch entry.Decoder {
case "json":
_ = json.Unmarshal(entry.Value, &spacePaths)
case "toml":
_ = toml.Unmarshal(entry.Value, &spacePaths)
case "xml":
_ = xml.Unmarshal(entry.Value, &spacePaths)
}
}
}
if entry, ok := o.Map["space_paths"]; ok {
_ = json.Unmarshal(entry.Value, &spacePaths)
// TODO log

if len(spacePaths) == 0 {
spacePaths[""] = r.ProviderPath
}
return spacePaths
}
Loading