From bf4645f4246853512647b6d4c5b7ff571959ced0 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 17 Dec 2021 11:52:28 +0100 Subject: [PATCH 1/2] fix valid points mentioned on review --- internal/grpc/interceptors/auth/scope.go | 21 -- internal/grpc/services/gateway/appprovider.go | 4 +- .../grpc/services/gateway/ocmshareprovider.go | 20 +- .../grpc/services/gateway/storageprovider.go | 196 +++++----------- .../services/gateway/usershareprovider.go | 20 +- .../services/gateway/webdavstorageprovider.go | 217 ------------------ .../publicstorageprovider.go | 18 +- .../sharesstorageprovider.go | 5 +- .../storageregistry/storageregistry.go | 19 +- internal/http/services/owncloud/ocdav/dav.go | 2 - .../handlers/apps/sharing/shares/pending.go | 15 +- .../apps/sharing/shares/pending_test.go | 2 +- .../handlers/apps/sharing/shares/shares.go | 10 +- .../apps/sharing/shares/shares_test.go | 2 +- internal/http/services/owncloud/ocs/ocs.go | 3 +- .../owncloud/ocs/response/response.go | 11 - pkg/rhttp/datatx/manager/spaces/spaces.go | 5 +- pkg/rhttp/datatx/utils/download/download.go | 2 +- pkg/sdk/common/opaque.go | 20 +- pkg/share/manager/json/json.go | 2 +- pkg/storage/fs/owncloud/owncloud.go | 1 - pkg/storage/registry/spaces/Readme.md | 10 +- pkg/storage/registry/spaces/spaces.go | 9 +- pkg/storage/utils/decomposedfs/spaces.go | 14 +- pkg/storage/utils/decomposedfs/tree/tree.go | 8 +- pkg/storage/utils/decomposedfs/upload.go | 1 - pkg/utils/utils.go | 14 +- 27 files changed, 172 insertions(+), 479 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 6222973c40..86d629fe35 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -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//some/path" - // // After the split we have [" ", "users", "/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) { diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go index ed34e5be5d..cc5f6bcb8d 100644 --- a/internal/grpc/services/gateway/appprovider.go +++ b/internal/grpc/services/gateway/appprovider.go @@ -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, }) @@ -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{ diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go index 071d1e30f9..69fab5e0b7 100644 --- a/internal/grpc/services/gateway/ocmshareprovider.go +++ b/internal/grpc/services/gateway/ocmshareprovider.go @@ -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, diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index e6b8861722..d737b9b666 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -21,6 +21,7 @@ package gateway import ( "context" "encoding/json" + "encoding/xml" "fmt" "net/url" "path" @@ -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" @@ -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 @@ -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, @@ -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, @@ -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, @@ -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, @@ -1108,12 +1104,8 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) 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, @@ -1183,16 +1175,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, @@ -1232,16 +1217,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, @@ -1395,20 +1373,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 @@ -1529,77 +1510,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 } @@ -1617,7 +1528,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, @@ -1627,14 +1538,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 } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 6db8c6caac..5a8bdb741a 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -379,16 +379,16 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource return status.NewInternal(ctx, "error finding storage provider") } - spaceID := "" - mountPath := providerInfo.ProviderPath - var root *provider.ResourceId - - spacePaths := decodeSpacePaths(providerInfo.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(providerInfo) { + mountPath = mp + rootSpace, rootNode, err := utils.SplitStorageSpaceID(spaceID) + if err != nil { + continue + } root = &provider.ResourceId{ StorageId: rootSpace, OpaqueId: rootNode, diff --git a/internal/grpc/services/gateway/webdavstorageprovider.go b/internal/grpc/services/gateway/webdavstorageprovider.go index 528c95bd3b..10782dba91 100644 --- a/internal/grpc/services/gateway/webdavstorageprovider.go +++ b/internal/grpc/services/gateway/webdavstorageprovider.go @@ -33,182 +33,6 @@ type webdavEndpoint struct { token string } -// The old logic had to check if a path pointed to the share folder a share mount point or a share child -// It also dealt with webdav references for OCM shares. The code below is commented en bloc to keep the -// old logic readable. -/* -func (s *svc) webdavRefStat(ctx context.Context, targetURL string, nameQueries ...string) (*provider.ResourceInfo, error) { - targetURL, err := appendNameQuery(targetURL, nameQueries...) - if err != nil { - return nil, err - } - - ep, err := s.extractEndpointInfo(ctx, targetURL) - if err != nil { - return nil, err - } - webdavEP, err := s.getWebdavEndpoint(ctx, ep.endpoint) - if err != nil { - return nil, err - } - - c := gowebdav.NewClient(webdavEP, "", "") - c.SetHeader(ctxpkg.TokenHeader, ep.token) - - // TODO(ishank011): We need to call PROPFIND ourselves as we need to retrieve - // ownloud-specific fields to get the resource ID and permissions. - info, err := c.Stat(ep.filePath) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("gateway: error statting %s at the webdav endpoint: %s", ep.filePath, webdavEP)) - } - return normalize(info.(*gowebdav.File)), nil -} - -func (s *svc) webdavRefLs(ctx context.Context, targetURL string, nameQueries ...string) ([]*provider.ResourceInfo, error) { - targetURL, err := appendNameQuery(targetURL, nameQueries...) - if err != nil { - return nil, err - } - - ep, err := s.extractEndpointInfo(ctx, targetURL) - if err != nil { - return nil, err - } - webdavEP, err := s.getWebdavEndpoint(ctx, ep.endpoint) - if err != nil { - return nil, err - } - - c := gowebdav.NewClient(webdavEP, "", "") - c.SetHeader(ctxpkg.TokenHeader, ep.token) - - // TODO(ishank011): We need to call PROPFIND ourselves as we need to retrieve - // ownloud-specific fields to get the resource ID and permissions. - infos, err := c.ReadDir(ep.filePath) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("gateway: error listing %s at the webdav endpoint: %s", ep.filePath, webdavEP)) - } - - mds := []*provider.ResourceInfo{} - for _, fi := range infos { - info := fi.(gowebdav.File) - mds = append(mds, normalize(&info)) - } - return mds, nil -} - -func (s *svc) webdavRefMkdir(ctx context.Context, targetURL string, nameQueries ...string) error { - targetURL, err := appendNameQuery(targetURL, nameQueries...) - if err != nil { - return err - } - - ep, err := s.extractEndpointInfo(ctx, targetURL) - if err != nil { - return err - } - webdavEP, err := s.getWebdavEndpoint(ctx, ep.endpoint) - if err != nil { - return err - } - - c := gowebdav.NewClient(webdavEP, "", "") - c.SetHeader(ctxpkg.TokenHeader, ep.token) - - err = c.Mkdir(ep.filePath, 0700) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("gateway: error creating dir %s at the webdav endpoint: %s", ep.filePath, webdavEP)) - } - return nil -} - -func (s *svc) webdavRefMove(ctx context.Context, targetURL, src, destination string) error { - srcURL, err := appendNameQuery(targetURL, src) - if err != nil { - return err - } - srcEP, err := s.extractEndpointInfo(ctx, srcURL) - if err != nil { - return err - } - srcWebdavEP, err := s.getWebdavEndpoint(ctx, srcEP.endpoint) - if err != nil { - return err - } - - destURL, err := appendNameQuery(targetURL, destination) - if err != nil { - return err - } - destEP, err := s.extractEndpointInfo(ctx, destURL) - if err != nil { - return err - } - - c := gowebdav.NewClient(srcWebdavEP, "", "") - c.SetHeader(ctxpkg.TokenHeader, srcEP.token) - - err = c.Rename(srcEP.filePath, destEP.filePath, true) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("gateway: error renaming %s to %s at the webdav endpoint: %s", srcEP.filePath, destEP.filePath, srcWebdavEP)) - } - return nil -} - -func (s *svc) webdavRefDelete(ctx context.Context, targetURL string, nameQueries ...string) error { - targetURL, err := appendNameQuery(targetURL, nameQueries...) - if err != nil { - return err - } - - ep, err := s.extractEndpointInfo(ctx, targetURL) - if err != nil { - return err - } - webdavEP, err := s.getWebdavEndpoint(ctx, ep.endpoint) - if err != nil { - return err - } - - c := gowebdav.NewClient(webdavEP, "", "") - c.SetHeader(ctxpkg.TokenHeader, ep.token) - - err = c.Remove(ep.filePath) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("gateway: error removing %s at the webdav endpoint: %s", ep.filePath, webdavEP)) - } - return nil -} - -func (s *svc) webdavRefTransferEndpoint(ctx context.Context, targetURL string, nameQueries ...string) (string, *types.Opaque, error) { - targetURL, err := appendNameQuery(targetURL, nameQueries...) - if err != nil { - return "", nil, err - } - - ep, err := s.extractEndpointInfo(ctx, targetURL) - if err != nil { - return "", nil, err - } - webdavEP, err := s.getWebdavEndpoint(ctx, ep.endpoint) - if err != nil { - return "", nil, err - } - - return webdavEP, &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "webdav-file-path": { - Decoder: "plain", - Value: []byte(ep.filePath), - }, - "webdav-token": { - Decoder: "plain", - Value: []byte(ep.token), - }, - }, - }, nil -} -*/ func (s *svc) extractEndpointInfo(ctx context.Context, targetURL string) (*webdavEndpoint, error) { if targetURL == "" { return nil, errtypes.BadRequest("gateway: ref target is an empty uri") @@ -234,47 +58,6 @@ func (s *svc) extractEndpointInfo(ctx context.Context, targetURL string) (*webda }, nil } -// The old logic had to check if a path pointed to the share folder a share mount point or a share child -// It also dealt with webdav references for OCM shares. The code below is commented en bloc to keep the -// old logic readable. -/* -func (s *svc) getWebdavEndpoint(ctx context.Context, domain string) (string, error) { - meshProvider, err := s.GetInfoByDomain(ctx, &ocmprovider.GetInfoByDomainRequest{ - Domain: domain, - }) - if err != nil { - return "", errors.Wrap(err, "gateway: error calling GetInfoByDomain") - } - for _, s := range meshProvider.ProviderInfo.Services { - if strings.ToLower(s.Endpoint.Type.Name) == "webdav" { - return s.Endpoint.Path, nil - } - } - return "", errtypes.NotFound(domain) -} - -func normalize(info *gowebdav.File) *provider.ResourceInfo { - return &provider.ResourceInfo{ - // TODO(ishank011): Add Id, PermissionSet, Owner - Path: info.Path(), - Type: getResourceType(info.IsDir()), - Etag: info.ETag(), - MimeType: info.ContentType(), - Size: uint64(info.Size()), - Mtime: &types.Timestamp{ - Seconds: uint64(info.ModTime().Unix()), - }, - } -} - -func getResourceType(isDir bool) provider.ResourceType { - if isDir { - return provider.ResourceType_RESOURCE_TYPE_CONTAINER - } - return provider.ResourceType_RESOURCE_TYPE_FILE -} -*/ - func appendNameQuery(targetURL string, nameQueries ...string) (string, error) { uri, err := url.Parse(targetURL) if err != nil { diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index c59471ac42..0ee727e2ee 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -44,6 +44,9 @@ import ( gstatus "google.golang.org/grpc/status" ) +// SpaceTypePublic is the public space type +var SpaceTypePublic = "public" + func init() { rgrpc.Register("publicstorageprovider", New) } @@ -309,13 +312,16 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora for _, f := range req.Filters { switch f.Type { case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: - if f.GetSpaceType() != "public" { + if f.GetSpaceType() != SpaceTypePublic { return &provider.ListStorageSpacesResponse{ Status: &rpc.Status{Code: rpc.Code_CODE_OK}, }, nil } case provider.ListStorageSpacesRequest_Filter_TYPE_ID: - spaceid, _ := utils.SplitStorageSpaceID(f.GetId().OpaqueId) + spaceid, _, err := utils.SplitStorageSpaceID(f.GetId().OpaqueId) + if err != nil { + continue + } if spaceid != utils.PublicStorageProviderID { return &provider.ListStorageSpacesResponse{ Status: &rpc.Status{Code: rpc.Code_CODE_OK}, @@ -330,7 +336,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora Id: &provider.StorageSpaceId{ OpaqueId: utils.PublicStorageProviderID, }, - SpaceType: "public", + SpaceType: SpaceTypePublic, // return the actual resource id? Root: &provider.ResourceId{ StorageId: utils.PublicStorageProviderID, @@ -664,8 +670,12 @@ func filterPermissions(l *provider.ResourcePermissions, r *provider.ResourcePerm } func (s *service) unwrap(ctx context.Context, ref *provider.Reference) (token string, relativePath string, err error) { + isValidReference := func(r *provider.Reference) bool { + return r != nil && r.ResourceId != nil && r.ResourceId.StorageId != "" && r.ResourceId.OpaqueId != "" + } + switch { - case ref == nil, ref.ResourceId == nil, ref.ResourceId.StorageId == "", ref.ResourceId.OpaqueId == "": + case !isValidReference(ref): return "", "", errtypes.BadRequest("resourceid required, got " + ref.String()) case ref.Path == "": // id based stat diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index c15c986d99..c7529ce6bd 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -313,7 +313,10 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora }, nil } case provider.ListStorageSpacesRequest_Filter_TYPE_ID: - spaceid, _ := utils.SplitStorageSpaceID(f.GetId().OpaqueId) + spaceid, _, err := utils.SplitStorageSpaceID(f.GetId().OpaqueId) + if err != nil { + continue + } if spaceid != utils.ShareStorageProviderID { return &provider.ListStorageSpacesResponse{ // a specific id was requested, return not found instead of empty list diff --git a/internal/grpc/services/storageregistry/storageregistry.go b/internal/grpc/services/storageregistry/storageregistry.go index e81bb6bd81..bab1bd2f17 100644 --- a/internal/grpc/services/storageregistry/storageregistry.go +++ b/internal/grpc/services/storageregistry/storageregistry.go @@ -21,8 +21,10 @@ package storageregistry import ( "context" "encoding/json" + "encoding/xml" "fmt" + "github.com/BurntSushi/toml" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registrypb "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" @@ -118,7 +120,6 @@ func (s *service) ListStorageProviders(ctx context.Context, req *registrypb.List return res, nil } -// FIXME rename to GetStorageProvider func (s *service) GetStorageProviders(ctx context.Context, req *registrypb.GetStorageProvidersRequest) (*registrypb.GetStorageProvidersResponse, error) { space, err := decodeSpace(req.Opaque) if err != nil { @@ -149,12 +150,20 @@ func (s *service) GetStorageProviders(ctx context.Context, req *registrypb.GetSt func decodeSpace(o *typespb.Opaque) (*provider.StorageSpace, error) { if entry, ok := o.Map["space"]; ok { - space := &provider.StorageSpace{} - if err := json.Unmarshal(entry.Value, space); err != nil { - return nil, err + var unmarshal func([]byte, interface{}) error + switch entry.Decoder { + case "json": + unmarshal = json.Unmarshal + case "toml": + unmarshal = toml.Unmarshal + case "xml": + unmarshal = xml.Unmarshal } - return space, nil + + space := &provider.StorageSpace{} + return space, unmarshal(entry.Value, space) } + return nil, fmt.Errorf("missing space in opaque property") } diff --git a/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go index daf45c732f..65af86f3a4 100644 --- a/internal/http/services/owncloud/ocdav/dav.go +++ b/internal/http/services/owncloud/ocdav/dav.go @@ -20,7 +20,6 @@ package ocdav import ( "context" - "fmt" "net/http" "path" "strings" @@ -98,7 +97,6 @@ func (h *DavHandler) Handler(s *svc) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() log := appctx.GetLogger(ctx) - log.Info().Str("request", fmt.Sprintf("%#v", r)).Msg("Got webdav request") // if there is no file in the request url we assume the request url is: "/remote.php/dav/files" // https://github.com/owncloud/core/blob/18475dac812064b21dabcc50f25ef3ffe55691a5/tests/acceptance/features/apiWebdavOperations/propfind.feature diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go index 1d8bad2724..b73615de5f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go @@ -41,14 +41,14 @@ import ( ) const ( - // shareID is the id of the share to update. It is present in the request URL. - shareID string = "shareid" + // shareidkey is the key user to obtain the id of the share to update. It is present in the request URL. + shareidkey string = "shareid" ) // AcceptReceivedShare handles Post Requests on /apps/files_sharing/api/v1/shares/{shareid} func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - shareID := chi.URLParam(r, shareID) + shareID := chi.URLParam(r, shareidkey) client, err := h.getClient() if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err) @@ -267,7 +267,14 @@ func getSharesList(ctx context.Context, client GatewayClient) (*collaboration.Li // arbitraryOcsResponse abstracts the boilerplate that is creating a response.Response struct. func arbitraryOcsResponse(statusCode int, message string) *response.Response { - r := response.NewResponse() + r := response.Response{ + OCS: &response.Payload{ + XMLName: struct{}{}, + Meta: response.Meta{}, + Data: nil, + }, + } + r.OCS.Meta.StatusCode = statusCode r.OCS.Meta.Message = message return &r diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go index 303ae7dc82..1412d19af5 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending_test.go @@ -58,7 +58,7 @@ var _ = Describe("The ocs API", func() { c := &config.Config{} c.Init() - h.Init(c, func() (shares.GatewayClient, error) { + h.InitWithGetter(c, func() (shares.GatewayClient, error) { return client, nil }) }) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 16d107a5ba..778e125c7f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -128,8 +128,8 @@ type GatewayClient interface { GetUserByClaim(ctx context.Context, in *userpb.GetUserByClaimRequest, opts ...grpc.CallOption) (*userpb.GetUserByClaimResponse, error) } -// InitDefault initializes the handler using default values -func (h *Handler) InitDefault(c *config.Config) { +// Init initializes the handler using default values +func (h *Handler) Init(c *config.Config) { h.gatewayAddr = c.GatewaySvc h.machineAuthAPIKey = c.MachineAuthAPIKey h.storageRegistryAddr = c.StorageregistrySvc @@ -153,9 +153,9 @@ func (h *Handler) InitDefault(c *config.Config) { h.getClient = h.getPoolClient } -// Init initializes the handler -func (h *Handler) Init(c *config.Config, clientGetter GatewayClientGetter) { - h.InitDefault(c) +// InitWithGetter initializes the handler and adds the clientGetter +func (h *Handler) InitWithGetter(c *config.Config, clientGetter GatewayClientGetter) { + h.Init(c) h.getClient = clientGetter } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go index f61dc5cf31..6c20a7a9e7 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go @@ -60,7 +60,7 @@ var _ = Describe("The ocs API", func() { c := &config.Config{} c.Init() - h.Init(c, func() (shares.GatewayClient, error) { + h.InitWithGetter(c, func() (shares.GatewayClient, error) { return client, nil }) }) diff --git a/internal/http/services/owncloud/ocs/ocs.go b/internal/http/services/owncloud/ocs/ocs.go index 6e08752114..5dc87fff9f 100644 --- a/internal/http/services/owncloud/ocs/ocs.go +++ b/internal/http/services/owncloud/ocs/ocs.go @@ -48,6 +48,7 @@ type svc struct { warmupCacheTracker *ttlcache.Cache } +// New initializes the service func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) { conf := &config.Config{} if err := mapstructure.Decode(m, conf); err != nil { @@ -96,7 +97,7 @@ func (s *svc) routerInit() error { capabilitiesHandler.Init(s.c) usersHandler.Init(s.c) configHandler.Init(s.c) - sharesHandler.InitDefault(s.c) + sharesHandler.Init(s.c) shareesHandler.Init(s.c) s.router.Route("/v{version:(1|2)}.php", func(r chi.Router) { diff --git a/internal/http/services/owncloud/ocs/response/response.go b/internal/http/services/owncloud/ocs/response/response.go index 85fc4f4cad..f92af90875 100644 --- a/internal/http/services/owncloud/ocs/response/response.go +++ b/internal/http/services/owncloud/ocs/response/response.go @@ -45,17 +45,6 @@ type Response struct { OCS *Payload `json:"ocs"` } -// NewResponse returns an empty response -func NewResponse() Response { - return Response{ - OCS: &Payload{ - XMLName: struct{}{}, - Meta: Meta{}, - Data: nil, - }, - } -} - // Payload combines response metadata and data type Payload struct { XMLName struct{} `json:"-" xml:"ocs"` diff --git a/pkg/rhttp/datatx/manager/spaces/spaces.go b/pkg/rhttp/datatx/manager/spaces/spaces.go index 120697e7dd..e451889f5e 100644 --- a/pkg/rhttp/datatx/manager/spaces/spaces.go +++ b/pkg/rhttp/datatx/manager/spaces/spaces.go @@ -82,10 +82,7 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { fn := path.Clean(strings.TrimLeft(r.URL.Path, "/")) defer r.Body.Close() - // TODO refactor: pass Reference to Upload & GetOrHeadFile - // build a storage space reference - storageid, opaqeid := utils.SplitStorageSpaceID(spaceID) - + storageid, opaqeid, _ := utils.SplitStorageSpaceID(spaceID) ref := &provider.Reference{ ResourceId: &provider.ResourceId{StorageId: storageid, OpaqueId: opaqeid}, Path: fn, diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go index 1062368bfa..21b28d3645 100644 --- a/pkg/rhttp/datatx/utils/download/download.go +++ b/pkg/rhttp/datatx/utils/download/download.go @@ -54,7 +54,7 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI ref = &provider.Reference{Path: path.Join("/", fn)} } else { // build a storage space reference - storageid, opaqeid := utils.SplitStorageSpaceID(spaceID) + storageid, opaqeid, _ := utils.SplitStorageSpaceID(spaceID) ref = &provider.Reference{ ResourceId: &provider.ResourceId{StorageId: storageid, OpaqueId: opaqeid}, // ensure the relative path starts with '.' diff --git a/pkg/sdk/common/opaque.go b/pkg/sdk/common/opaque.go index fc182538a4..b9cf1f01ae 100644 --- a/pkg/sdk/common/opaque.go +++ b/pkg/sdk/common/opaque.go @@ -19,21 +19,35 @@ package common import ( + "encoding/json" + "encoding/xml" "fmt" + "github.com/BurntSushi/toml" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" ) // DecodeOpaqueMap decodes a Reva opaque object into a map of strings. -// Only plain decoders are currently supported. func DecodeOpaqueMap(opaque *types.Opaque) map[string]string { entries := make(map[string]string) if opaque != nil { for k, v := range opaque.GetMap() { - // Only plain values are currently supported - if v.Decoder == "plain" { + switch v.Decoder { + case "plain": entries[k] = string(v.Value) + case "json": + var s string + _ = json.Unmarshal(v.Value, &s) + entries[k] = s + case "toml": + var s string + _ = toml.Unmarshal(v.Value, &s) + entries[k] = s + case "xml": + var s string + _ = xml.Unmarshal(v.Value, &s) + entries[k] = s } } } diff --git a/pkg/share/manager/json/json.go b/pkg/share/manager/json/json.go index 9347fa613b..aa3c946e7b 100644 --- a/pkg/share/manager/json/json.go +++ b/pkg/share/manager/json/json.go @@ -400,7 +400,7 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F } // if there is a mix-up of shares of type group and shares of type user we need to deduplicate them, since it points - // to the same resource. Leave the more explicit and hide the more explicit. In this case we hide the group shares + // to the same resource. Leave the more explicit and hide the less explicit. In this case we hide the group shares // and return the user share to the user. filtered := make([]*collaboration.ReceivedShare, 0) filtered = append(filtered, rss...) diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go index 95877c9ecd..5a0c0ee50d 100644 --- a/pkg/storage/fs/owncloud/owncloud.go +++ b/pkg/storage/fs/owncloud/owncloud.go @@ -79,7 +79,6 @@ const ( favoriteKey string = "http://owncloud.org/ns/favorite" spaceTypeAny = "*" - // spaceIDAny = "*" ) var defaultPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{ diff --git a/pkg/storage/registry/spaces/Readme.md b/pkg/storage/registry/spaces/Readme.md index 9f1bd88f6e..4fa3741a50 100644 --- a/pkg/storage/registry/spaces/Readme.md +++ b/pkg/storage/registry/spaces/Readme.md @@ -2,13 +2,13 @@ The spaces registry recognizes individual spaces instead of storage providers. While it is configured with a list of storage providers, it will query them for all storage spaces and use the space ids to resolve id based lookups. -Furthermore, path based lookups will take into account a path templvate to present a human readable file tree. +Furthermore, path based lookups will take into account a path template to present a human readable file tree. ## Configuration The spaces registry takes two configuration options: -1. `home_template` is used to buld a path for the users home. It uses a template that can access the current user in the context, e.g. `/users/{{.Id.OpaqueId}}` +1. `home_template` is used to build a path for the users home. It uses a template that can access the current user in the context, e.g. `/users/{{.Id.OpaqueId}}` 2. `rules` is a map of path patterns to config rules ### Patterns @@ -20,11 +20,9 @@ The pattern is used when matching the path for path based requests. A rule has several properties: -* `mapping` unused? * `address` The ip address of the CS3 storage provider -* `path_template` TODO -> rename to space\_path or space\_mount\_point -* `aliases` unused? -* `allowed_user_agents` unused? FIXME this seems to be used to route requests based on user agent +* `path_template` Will be renamed to space\_path or space\_mount\_point. If set this must be a valid go template. +* `allowed_user_agents` FIXME this seems to be used to route requests based on user agent It also carries filters that are sent with a ListStorageSpaces call to a storage provider diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index e9801086d8..53eaeec051 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -74,7 +74,8 @@ func (sc *spaceConfig) SpacePath(currentUser *userpb.User, space *providerpb.Sto return b.String(), nil } -type provider struct { +// Provider holds information on Spaces +type Provider struct { // Spaces is a map from space type to space config Spaces map[string]*spaceConfig `mapstructure:"spaces"` } @@ -90,7 +91,7 @@ type StorageProviderClient interface { } type config struct { - Providers map[string]*provider `mapstructure:"providers"` + Providers map[string]*Provider `mapstructure:"providers"` HomeTemplate string `mapstructure:"home_template"` } @@ -101,7 +102,7 @@ func (c *config) init() { } if len(c.Providers) == 0 { - c.Providers = map[string]*provider{ + c.Providers = map[string]*Provider{ sharedconf.GetGatewaySVC(""): { Spaces: map[string]*spaceConfig{ "personal": {MountPoint: "/users", PathTemplate: "/users/{{.Space.Owner.Id.OpaqueId}}"}, @@ -333,7 +334,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r } p.Opaque, err = spacePathsToOpaque(spacePaths) if err != nil { - appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing") + appctx.GetLogger(ctx).Error().Err(err).Msg("marshaling space paths map failed, continuing") continue } // we can stop after we found the first space diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index d28ac55257..fefc797bd8 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -187,7 +187,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide case provider.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE: spaceTypes = append(spaceTypes, filter[i].GetSpaceType()) case provider.ListStorageSpacesRequest_Filter_TYPE_ID: - spaceID, nodeID = utils.SplitStorageSpaceID(filter[i].GetId().OpaqueId) + spaceID, nodeID, _ = utils.SplitStorageSpaceID(filter[i].GetId().OpaqueId) } } if len(spaceTypes) == 0 { @@ -290,7 +290,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorageSpaceRequest) (*provider.UpdateStorageSpaceResponse, error) { space := req.StorageSpace - _, spaceID := utils.SplitStorageSpaceID(space.Id.OpaqueId) + _, spaceID, _ := utils.SplitStorageSpaceID(space.Id.OpaqueId) matches, err := filepath.Glob(filepath.Join(fs.o.Root, "spaces", spaceTypeAny, spaceID)) if err != nil { @@ -437,16 +437,6 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, // Mtime is set either as node.tmtime or as fi.mtime below } - // filter out spaces user cannot access (currently based on stat permission) - // p, err := n.ReadUserPermissions(ctx, user) - // if err != nil { - // return nil, err - // } - - // if !(canListAllSpaces || p.Stat) { - // return nil, - // } - user := ctxpkg.ContextMustGetUser(ctx) _, canListAllSpaces := permissions["list-all-spaces"] if !canListAllSpaces { diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index a2507a1e94..b105ddd0f9 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -240,11 +240,6 @@ func (t *Tree) CreateDir(ctx context.Context, n *node.Node) (err error) { return errtypes.AlreadyExists(n.ID) // path? } - // Allow passing in the node id - // if n.ID != "" { - // TODO check if already exists - // } - // create a directory node if n.ID == "" { n.ID = uuid.New().String() @@ -275,7 +270,6 @@ func (t *Tree) CreateDir(ctx context.Context, n *node.Node) (err error) { return } - // try to remove the node e := t.Delete(ctx, n) switch { case e != nil: @@ -285,7 +279,7 @@ func (t *Tree) CreateDir(ctx context.Context, n *node.Node) (err error) { if e == nil { e = rm() if e != nil { - appctx.GetLogger(ctx).Debug().Err(e).Msg("cannot purge from trashcan") + appctx.GetLogger(ctx).Debug().Err(e).Msg("cannot purge from trashbin") } } } diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index f9f2a1bf99..21775b05bf 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -186,7 +186,6 @@ func (fs *Decomposedfs) NewUpload(ctx context.Context, info tusd.FileInfo) (uplo log := appctx.GetLogger(ctx) log.Debug().Interface("info", info).Msg("Decomposedfs: NewUpload") - // sanity checks if info.MetaData["filename"] == "" { return nil, errors.New("Decomposedfs: missing filename in metadata") } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 1f937ade58..9134f7eaa7 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -19,6 +19,7 @@ package utils import ( + "errors" "math/rand" "net" "net/http" @@ -309,15 +310,15 @@ func UserTypeToString(accountType userpb.UserType) string { // SplitStorageSpaceID can be used to split `storagespaceid` into `storageid` and `nodeid`. // If no specific node is appended with a `!` separator the spaceid is used as nodeid, identifying the root of the space. -func SplitStorageSpaceID(ssid string) (storageid, nodeid string) { +func SplitStorageSpaceID(ssid string) (storageid, nodeid string, err error) { if ssid == "" { - return "", "" + return "", "", errors.New("can't split empty StorageSpaceID") } parts := strings.SplitN(ssid, "!", 2) if len(parts) == 1 { - return parts[0], parts[0] + return parts[0], parts[0], nil } - return parts[0], parts[1] + return parts[0], parts[1], nil } // ParseStorageSpaceReference parses a string into a spaces reference. @@ -325,7 +326,10 @@ func SplitStorageSpaceID(ssid string) (storageid, nodeid string) { func ParseStorageSpaceReference(sRef string) (provider.Reference, error) { parts := strings.SplitN(sRef, "/", 2) - storageid, nodeid := SplitStorageSpaceID(parts[0]) + storageid, nodeid, err := SplitStorageSpaceID(parts[0]) + if err != nil { + return provider.Reference{}, err + } var relPath string if len(parts) == 2 { From 2e36d9710814b9fc63bfa6d498d84fd16ee386bb Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 17 Dec 2021 14:26:50 +0100 Subject: [PATCH 2/2] fix linting Signed-off-by: jkoberg --- internal/grpc/services/gateway/storageprovider.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index d737b9b666..a3ef0edf3d 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1100,11 +1100,8 @@ func (s *svc) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) }, nil } - spaceID := "" - mountPath := providerInfos[i].ProviderPath var root *provider.ResourceId - - for spaceID, mountPath = range decodeSpacePaths(providerInfos[i]) { + for spaceID, mountPath := range decodeSpacePaths(providerInfos[i]) { rootSpace, rootNode, _ := utils.SplitStorageSpaceID(spaceID) root = &provider.ResourceId{ StorageId: rootSpace,