From 94a197515e74ea3d7d08d456a55761626f276c0e Mon Sep 17 00:00:00 2001 From: Niraj Acharya Date: Fri, 1 Mar 2024 14:15:51 +0545 Subject: [PATCH 01/16] bump latest ocis commit id --- .drone.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.env b/.drone.env index 2cbead4a7c5..81f9c725e2e 100644 --- a/.drone.env +++ b/.drone.env @@ -1,4 +1,4 @@ # The test runner source for API tests -APITESTS_COMMITID=c13aefed846efe4c1a180e12e004a494bc10fe93 +APITESTS_COMMITID=d33935c25756878c10e28b28240aade7e2b79da0 APITESTS_BRANCH=master APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git From 4b1c69a6fffce8b6e4c1a677ad0ff47dcf7cb0ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 1 Mar 2024 09:42:27 +0100 Subject: [PATCH 02/16] Kill service spanning stat cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../kill-service-spanning-stat-cache.md | 5 ++ internal/grpc/services/gateway/gateway.go | 12 --- .../grpc/services/gateway/ocmshareprovider.go | 14 ++- .../services/gateway/publicshareprovider.go | 34 +------- .../grpc/services/gateway/storageprovider.go | 17 ---- .../services/gateway/storageprovidercache.go | 85 +++++++------------ .../services/gateway/usershareprovider.go | 10 +-- pkg/rhttp/datatx/datatx.go | 6 -- pkg/rhttp/datatx/manager/simple/simple.go | 3 - pkg/rhttp/datatx/manager/spaces/spaces.go | 3 - pkg/rhttp/datatx/manager/tus/tus.go | 5 -- .../utils/decomposedfs/decomposedfs.go | 9 -- .../utils/decomposedfs/options/options.go | 5 +- pkg/storage/utils/decomposedfs/tree/tree.go | 1 + 14 files changed, 48 insertions(+), 161 deletions(-) create mode 100644 changelog/unreleased/kill-service-spanning-stat-cache.md diff --git a/changelog/unreleased/kill-service-spanning-stat-cache.md b/changelog/unreleased/kill-service-spanning-stat-cache.md new file mode 100644 index 00000000000..5afb0539728 --- /dev/null +++ b/changelog/unreleased/kill-service-spanning-stat-cache.md @@ -0,0 +1,5 @@ +Change: Drop unused service spanning stat cache + +We removed the stat cache shared between gateway and storage providers. It is constantly invalidated and needs a different approach. + +https://github.com/cs3org/reva/pull/4542 diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go index b8e6533da96..505afa0c6d0 100644 --- a/internal/grpc/services/gateway/gateway.go +++ b/internal/grpc/services/gateway/gateway.go @@ -67,7 +67,6 @@ type config struct { DataTransfersFolder string `mapstructure:"data_transfers_folder"` TokenManagers map[string]map[string]interface{} `mapstructure:"token_managers"` AllowedUserAgents map[string][]string `mapstructure:"allowed_user_agents"` // map[path][]user-agent - StatCacheConfig cache.Config `mapstructure:"stat_cache_config"` CreatePersonalSpaceCacheConfig cache.Config `mapstructure:"create_personal_space_cache_config"` ProviderCacheConfig cache.Config `mapstructure:"provider_cache_config"` UseCommonSpaceRootShareLogic bool `mapstructure:"use_common_space_root_share_logic"` @@ -117,14 +116,6 @@ func (c *config) init() { } // caching needs to be explicitly enabled - if c.StatCacheConfig.Store == "" { - c.StatCacheConfig.Store = "noop" - } - - if c.StatCacheConfig.Database == "" { - c.StatCacheConfig.Database = "reva" - } - if c.ProviderCacheConfig.Store == "" { c.ProviderCacheConfig.Store = "noop" } @@ -146,7 +137,6 @@ type svc struct { c *config dataGatewayURL url.URL tokenmgr token.Manager - statCache cache.StatCache providerCache cache.ProviderCache createPersonalSpaceCache cache.CreatePersonalSpaceCache } @@ -177,7 +167,6 @@ func New(m map[string]interface{}, _ *grpc.Server) (rgrpc.Service, error) { c: c, dataGatewayURL: *u, tokenmgr: tokenManager, - statCache: cache.GetStatCache(c.StatCacheConfig), providerCache: cache.GetProviderCache(c.ProviderCacheConfig), createPersonalSpaceCache: cache.GetCreatePersonalSpaceCache(c.CreatePersonalSpaceCacheConfig), } @@ -190,7 +179,6 @@ func (s *svc) Register(ss *grpc.Server) { } func (s *svc) Close() error { - s.statCache.Close() s.providerCache.Close() s.createPersonalSpaceCache.Close() return nil diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go index 6ee1182c4e5..3cdac238d26 100644 --- a/internal/grpc/services/gateway/ocmshareprovider.go +++ b/internal/grpc/services/gateway/ocmshareprovider.go @@ -52,21 +52,17 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest } status, err := s.addGrant(ctx, req.ResourceId, req.Grantee, req.AccessMethods[0].GetWebdavOptions().Permissions, req.Expiration, nil) - if err != nil { + switch { + case err != nil: appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg(err.Error()) return nil, errors.Wrap(err, "gateway: error adding grant to storage") - } - - switch status.Code { - case rpc.Code_CODE_OK: - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.ResourceId) - case rpc.Code_CODE_UNIMPLEMENTED: + case status.Code == rpc.Code_CODE_UNIMPLEMENTED: appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants not supported, ignoring") - default: + case status.Code != rpc.Code_CODE_OK: appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants is not successful") return &ocm.CreateOCMShareResponse{ Status: status, - }, err + }, nil } return res, nil diff --git a/internal/grpc/services/gateway/publicshareprovider.go b/internal/grpc/services/gateway/publicshareprovider.go index 7a683f18902..4738acd7c54 100644 --- a/internal/grpc/services/gateway/publicshareprovider.go +++ b/internal/grpc/services/gateway/publicshareprovider.go @@ -21,11 +21,9 @@ package gateway import ( "context" - userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" - ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/pkg/errors" ) @@ -39,15 +37,7 @@ func (s *svc) CreatePublicShare(ctx context.Context, req *link.CreatePublicShare return nil, err } - res, err := c.CreatePublicShare(ctx, req) - if err != nil { - return nil, err - } - - if res.GetShare() != nil { - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), res.Share.ResourceId) - } - return res, nil + return c.CreatePublicShare(ctx, req) } func (s *svc) RemovePublicShare(ctx context.Context, req *link.RemovePublicShareRequest) (*link.RemovePublicShareResponse, error) { @@ -58,13 +48,7 @@ func (s *svc) RemovePublicShare(ctx context.Context, req *link.RemovePublicShare if err != nil { return nil, err } - res, err := driver.RemovePublicShare(ctx, req) - if err != nil { - return nil, err - } - // TODO: How to find out the resourceId? -> get public share first, then delete - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), nil) - return res, nil + return driver.RemovePublicShare(ctx, req) } func (s *svc) GetPublicShareByToken(ctx context.Context, req *link.GetPublicShareByTokenRequest) (*link.GetPublicShareByTokenResponse, error) { @@ -137,17 +121,5 @@ func (s *svc) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicShare }, nil } - res, err := pClient.UpdatePublicShare(ctx, req) - if err != nil { - return nil, errors.Wrap(err, "error updating share") - } - if res.GetShare() != nil { - s.statCache.RemoveStatContext(ctx, - &userprovider.UserId{ - OpaqueId: res.Share.Owner.GetOpaqueId(), - }, - res.Share.ResourceId, - ) - } - return res, nil + return pClient.UpdatePublicShare(ctx, req) } diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index cf3a7387e3e..781295963da 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -326,7 +326,6 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag if res.Status.Code == rpc.Code_CODE_OK { id := res.StorageSpace.Root - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), id) s.providerCache.RemoveListStorageProviders(id) } return res, nil @@ -363,7 +362,6 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag } id := &provider.ResourceId{OpaqueId: req.Id.OpaqueId} - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), id) s.providerCache.RemoveListStorageProviders(id) if dsRes.Status.Code != rpc.Code_CODE_OK { @@ -608,7 +606,6 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile } } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return &gateway.InitiateFileUploadResponse{ Opaque: storageRes.Opaque, Status: storageRes.Status, @@ -645,7 +642,6 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -688,7 +684,6 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -715,8 +710,6 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo req.Source = sref req.Destination = dref - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Source.ResourceId) - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Destination.ResourceId) return c.Move(ctx, req) } @@ -739,7 +732,6 @@ func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitra return nil, errors.Wrap(err, "gateway: error calling SetArbitraryMetadata") } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -761,7 +753,6 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb } return nil, errors.Wrap(err, "gateway: error calling UnsetArbitraryMetadata") } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -785,7 +776,6 @@ func (s *svc) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provi return nil, errors.Wrap(err, "gateway: error calling SetLock") } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -826,7 +816,6 @@ func (s *svc) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) return nil, errors.Wrap(err, "gateway: error calling RefreshLock") } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -847,12 +836,10 @@ func (s *svc) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provide return nil, errors.Wrap(err, "gateway: error calling Unlock") } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } // Stat returns the Resoure info for a given resource by forwarding the request to the responsible provider. -// TODO cache info func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { c, _, ref, err := s.findAndUnwrapUnique(ctx, req.Ref) if err != nil { @@ -927,7 +914,6 @@ func (s *svc) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileV }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -983,7 +969,6 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -1006,7 +991,6 @@ func (s *svc) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleReques }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Ref.ResourceId) return res, nil } @@ -1106,7 +1090,6 @@ func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderIn return &cachedAPIClient{ c: c, - statCache: s.statCache, createPersonalSpaceCache: s.createPersonalSpaceCache, }, nil } diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go index 8dd5d720b5e..f3db156ec2d 100644 --- a/internal/grpc/services/gateway/storageprovidercache.go +++ b/internal/grpc/services/gateway/storageprovidercache.go @@ -20,7 +20,6 @@ package gateway import ( "context" - "strings" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -84,40 +83,9 @@ func (c *cachedRegistryClient) GetHome(ctx context.Context, in *registry.GetHome type cachedAPIClient struct { c provider.ProviderAPIClient - statCache cache.StatCache createPersonalSpaceCache cache.CreatePersonalSpaceCache } -// Stat looks in cache first before forwarding to storage provider -func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) { - key := c.statCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId(), in.GetRef(), in.GetArbitraryMetadataKeys(), in.GetFieldMask().GetPaths()) - if key != "" { - s := &provider.StatResponse{} - if err := c.statCache.PullFromCache(key, s); err == nil { - return s, nil - } - } - - resp, err := c.c.Stat(ctx, in, opts...) - switch { - case err != nil: - return nil, err - case resp.Status.Code != rpc.Code_CODE_OK: - return resp, nil - case key == "": - return resp, nil - case strings.Contains(key, "sid:"+utils.ShareStorageProviderID): - // We cannot cache shares at the moment: - // we do not know when to invalidate them - // FIXME: find a way to cache/invalidate them too - return resp, nil - case utils.ReadPlainFromOpaque(resp.GetInfo().GetOpaque(), "status") == "processing": - return resp, nil - default: - return resp, c.statCache.PushToCache(key, resp) - } -} - // CreateHome caches calls to CreateHome locally - anyways they only need to be called once per user func (c *cachedAPIClient) CreateHome(ctx context.Context, in *provider.CreateHomeRequest, opts ...grpc.CallOption) (*provider.CreateHomeResponse, error) { key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId()) @@ -140,8 +108,37 @@ func (c *cachedAPIClient) CreateHome(ctx context.Context, in *provider.CreateHom } } +// CreateStorageSpace creates a storage space +func (c *cachedAPIClient) CreateStorageSpace(ctx context.Context, in *provider.CreateStorageSpaceRequest, opts ...grpc.CallOption) (*provider.CreateStorageSpaceResponse, error) { + if in.Type == "personal" { + key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId()) + if key != "" { + s := &provider.CreateStorageSpaceResponse{} + if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil { + return s, nil + } + } + resp, err := c.c.CreateStorageSpace(ctx, in, opts...) + switch { + case err != nil: + return nil, err + case resp.Status.Code != rpc.Code_CODE_OK && resp.Status.Code != rpc.Code_CODE_ALREADY_EXISTS: + return resp, nil + case key == "": + return resp, nil + default: + return resp, c.createPersonalSpaceCache.PushToCache(key, resp) + } + } + return c.c.CreateStorageSpace(ctx, in, opts...) +} + // methods below here are not cached, they just call the client directly +// Stat returns the Resoure info for a given resource +func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, opts ...grpc.CallOption) (*provider.StatResponse, error) { + return c.c.Stat(ctx, in, opts...) +} func (c *cachedAPIClient) AddGrant(ctx context.Context, in *provider.AddGrantRequest, opts ...grpc.CallOption) (*provider.AddGrantResponse, error) { return c.c.AddGrant(ctx, in, opts...) } @@ -229,29 +226,6 @@ func (c *cachedAPIClient) Unlock(ctx context.Context, in *provider.UnlockRequest func (c *cachedAPIClient) GetHome(ctx context.Context, in *provider.GetHomeRequest, opts ...grpc.CallOption) (*provider.GetHomeResponse, error) { return c.c.GetHome(ctx, in, opts...) } -func (c *cachedAPIClient) CreateStorageSpace(ctx context.Context, in *provider.CreateStorageSpaceRequest, opts ...grpc.CallOption) (*provider.CreateStorageSpaceResponse, error) { - if in.Type == "personal" { - key := c.createPersonalSpaceCache.GetKey(ctxpkg.ContextMustGetUser(ctx).GetId()) - if key != "" { - s := &provider.CreateStorageSpaceResponse{} - if err := c.createPersonalSpaceCache.PullFromCache(key, s); err == nil { - return s, nil - } - } - resp, err := c.c.CreateStorageSpace(ctx, in, opts...) - switch { - case err != nil: - return nil, err - case resp.Status.Code != rpc.Code_CODE_OK && resp.Status.Code != rpc.Code_CODE_ALREADY_EXISTS: - return resp, nil - case key == "": - return resp, nil - default: - return resp, c.createPersonalSpaceCache.PushToCache(key, resp) - } - } - return c.c.CreateStorageSpace(ctx, in, opts...) -} func (c *cachedAPIClient) ListStorageSpaces(ctx context.Context, in *provider.ListStorageSpacesRequest, opts ...grpc.CallOption) (*provider.ListStorageSpacesResponse, error) { return c.c.ListStorageSpaces(ctx, in, opts...) } @@ -261,7 +235,6 @@ func (c *cachedAPIClient) UpdateStorageSpace(ctx context.Context, in *provider.U func (c *cachedAPIClient) DeleteStorageSpace(ctx context.Context, in *provider.DeleteStorageSpaceRequest, opts ...grpc.CallOption) (*provider.DeleteStorageSpaceResponse, error) { return c.c.DeleteStorageSpace(ctx, in, opts...) } - func (c *cachedAPIClient) TouchFile(ctx context.Context, in *provider.TouchFileRequest, opts ...grpc.CallOption) (*provider.TouchFileResponse, error) { return c.c.TouchFile(ctx, in, opts...) } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 9a27f8f9c55..43f65d8c6a7 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -154,7 +154,6 @@ func (s *svc) updateShare(ctx context.Context, req *collaboration.UpdateShareReq } } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), res.Share.ResourceId) return res, nil } @@ -198,7 +197,6 @@ func (s *svc) updateSpaceShare(ctx context.Context, req *collaboration.UpdateSha return res, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.GetShare().GetResourceId()) s.providerCache.RemoveListStorageProviders(req.GetShare().GetResourceId()) return res, nil } @@ -282,7 +280,6 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update }, nil } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.Share.Share.ResourceId) return c.UpdateReceivedShare(ctx, req) /* TODO: Leftover from master merge. Do we need this? @@ -551,7 +548,7 @@ func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareReques } if s.c.CommitShareToStorageGrant { - // If the share is a denial we call denyGrant instead. + // If the share is a denial we call denyGrant instead. var status *rpc.Status if grants.PermissionsEqual(req.Grant.Permissions.Permissions, &provider.ResourcePermissions{}) { status, err = s.denyGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, nil) @@ -569,7 +566,7 @@ func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareReques switch status.Code { case rpc.Code_CODE_OK: - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.ResourceInfo.Id) + // ok case rpc.Code_CODE_UNIMPLEMENTED: appctx.GetLogger(ctx).Debug().Interface("status", status).Interface("req", req).Msg("storing grants not supported, ignoring") rollBackFn(status) @@ -613,7 +610,6 @@ func (s *svc) addSpaceShare(ctx context.Context, req *collaboration.CreateShareR switch st.Code { case rpc.Code_CODE_OK: - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), req.ResourceInfo.Id) s.providerCache.RemoveListStorageProviders(req.ResourceInfo.Id) case rpc.Code_CODE_UNIMPLEMENTED: appctx.GetLogger(ctx).Debug().Interface("status", st).Interface("req", req).Msg("storing grants not supported, ignoring") @@ -692,7 +688,6 @@ func (s *svc) removeShare(ctx context.Context, req *collaboration.RemoveShareReq } } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), share.ResourceId) return res, nil } @@ -725,7 +720,6 @@ func (s *svc) removeSpaceShare(ctx context.Context, ref *provider.ResourceId, gr Status: removeGrantStatus, }, err } - s.statCache.RemoveStatContext(ctx, ctxpkg.ContextMustGetUser(ctx).GetId(), ref) s.providerCache.RemoveListStorageProviders(ref) return &collaboration.RemoveShareResponse{Status: status.NewOK(ctx)}, nil } diff --git a/pkg/rhttp/datatx/datatx.go b/pkg/rhttp/datatx/datatx.go index 23328aadd96..cdbb27052e4 100644 --- a/pkg/rhttp/datatx/datatx.go +++ b/pkg/rhttp/datatx/datatx.go @@ -28,7 +28,6 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/pkg/events" "github.com/cs3org/reva/v2/pkg/storage" - "github.com/cs3org/reva/v2/pkg/storage/cache" "github.com/cs3org/reva/v2/pkg/utils" ) @@ -53,8 +52,3 @@ func EmitFileUploadedEvent(spaceOwnerOrManager, executant *userv1beta1.UserId, r return events.Publish(context.Background(), publisher, uploadedEv) } - -// InvalidateCache is a helper function which invalidates the stat cache -func InvalidateCache(owner *userv1beta1.UserId, ref *provider.Reference, statCache cache.StatCache) { - statCache.RemoveStatContext(context.TODO(), owner, ref.GetResourceId()) -} diff --git a/pkg/rhttp/datatx/manager/simple/simple.go b/pkg/rhttp/datatx/manager/simple/simple.go index 6699c9f77a2..607d6336adc 100644 --- a/pkg/rhttp/datatx/manager/simple/simple.go +++ b/pkg/rhttp/datatx/manager/simple/simple.go @@ -48,7 +48,6 @@ func init() { type manager struct { conf *cache.Config publisher events.Publisher - statCache cache.StatCache } func parseConfig(m map[string]interface{}) (*cache.Config, error) { @@ -70,7 +69,6 @@ func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, e return &manager{ conf: c, publisher: publisher, - statCache: cache.GetStatCache(*c), }, nil } @@ -110,7 +108,6 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { Body: r.Body, Length: r.ContentLength, }, func(spaceOwner, owner *userpb.UserId, ref *provider.Reference) { - datatx.InvalidateCache(owner, ref, m.statCache) if err := datatx.EmitFileUploadedEvent(spaceOwner, owner, ref, m.publisher); err != nil { sublog.Error().Err(err).Msg("failed to publish FileUploaded event") } diff --git a/pkg/rhttp/datatx/manager/spaces/spaces.go b/pkg/rhttp/datatx/manager/spaces/spaces.go index d39b12baa53..ce585ab5426 100644 --- a/pkg/rhttp/datatx/manager/spaces/spaces.go +++ b/pkg/rhttp/datatx/manager/spaces/spaces.go @@ -50,7 +50,6 @@ func init() { type manager struct { conf *cache.Config publisher events.Publisher - statCache cache.StatCache } func parseConfig(m map[string]interface{}) (*cache.Config, error) { @@ -72,7 +71,6 @@ func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, e return &manager{ conf: c, publisher: publisher, - statCache: cache.GetStatCache(*c), }, nil } @@ -117,7 +115,6 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { Body: r.Body, Length: r.ContentLength, }, func(spaceOwner, owner *userpb.UserId, ref *provider.Reference) { - datatx.InvalidateCache(owner, ref, m.statCache) if err := datatx.EmitFileUploadedEvent(spaceOwner, owner, ref, m.publisher); err != nil { sublog.Error().Err(err).Msg("failed to publish FileUploaded event") } diff --git a/pkg/rhttp/datatx/manager/tus/tus.go b/pkg/rhttp/datatx/manager/tus/tus.go index e9b2a07b039..f86b100866c 100644 --- a/pkg/rhttp/datatx/manager/tus/tus.go +++ b/pkg/rhttp/datatx/manager/tus/tus.go @@ -37,7 +37,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rhttp/datatx/manager/registry" "github.com/cs3org/reva/v2/pkg/rhttp/datatx/metrics" "github.com/cs3org/reva/v2/pkg/storage" - "github.com/cs3org/reva/v2/pkg/storage/cache" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/mitchellh/mapstructure" ) @@ -47,7 +46,6 @@ func init() { } type TusConfig struct { - cache.Config CorsEnabled bool `mapstructure:"cors_enabled"` CorsAllowOrigin string `mapstructure:"cors_allow_origin"` CorsAllowCredentials bool `mapstructure:"cors_allow_credentials"` @@ -60,7 +58,6 @@ type TusConfig struct { type manager struct { conf *TusConfig publisher events.Publisher - statCache cache.StatCache } func parseConfig(m map[string]interface{}) (*TusConfig, error) { @@ -81,7 +78,6 @@ func New(m map[string]interface{}, publisher events.Publisher) (datatx.DataTX, e return &manager{ conf: c, publisher: publisher, - statCache: cache.GetStatCache(c.Config), }, nil } @@ -142,7 +138,6 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { up := ups[0] executant := up.Executant() ref := up.Reference() - datatx.InvalidateCache(&executant, &ref, m.statCache) if m.publisher != nil { if err := datatx.EmitFileUploadedEvent(up.SpaceOwner(), &executant, &ref, m.publisher); err != nil { appctx.GetLogger(context.Background()).Error().Err(err).Msg("failed to publish FileUploaded event") diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 35cc29663fc..f06a50f80ad 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -40,7 +40,6 @@ import ( "github.com/cs3org/reva/v2/pkg/rhttp/datatx/metrics" "github.com/cs3org/reva/v2/pkg/rhttp/datatx/utils/download" "github.com/cs3org/reva/v2/pkg/storage" - "github.com/cs3org/reva/v2/pkg/storage/cache" "github.com/cs3org/reva/v2/pkg/storage/utils/chunking" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/aspects" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/lookup" @@ -108,7 +107,6 @@ type Decomposedfs struct { p permissions.Permissions chunkHandler *chunking.ChunkHandler stream events.Stream - cache cache.StatCache sessionStore SessionStore UserCache *ttlcache.Cache @@ -209,7 +207,6 @@ func New(o *options.Options, aspects aspects.Aspects) (storage.FS, error) { p: aspects.Permissions, chunkHandler: chunking.NewChunkHandler(filepath.Join(o.Root, "uploads")), stream: aspects.EventStream, - cache: cache.GetStatCache(o.StatCache), UserCache: ttlcache.NewCache(), userSpaceIndex: userSpaceIndex, groupSpaceIndex: groupSpaceIndex, @@ -324,9 +321,6 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) { fs.sessionStore.Cleanup(ctx, session, revertNodeMetadata, keepUpload, unmarkPostprocessing) - // remove cache entry in gateway - fs.cache.RemoveStatContext(ctx, ev.ExecutingUser.GetId(), &provider.ResourceId{SpaceId: n.SpaceID, OpaqueId: n.ID}) - if err := events.Publish( ctx, fs.stream, @@ -494,9 +488,6 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) { } metrics.UploadSessionsScanned.Inc() - - // remove cache entry in gateway - fs.cache.RemoveStatContext(ctx, ev.ExecutingUser.GetId(), &provider.ResourceId{SpaceId: n.SpaceID, OpaqueId: n.ID}) default: log.Error().Interface("event", ev).Msg("Unknown event") } diff --git a/pkg/storage/utils/decomposedfs/options/options.go b/pkg/storage/utils/decomposedfs/options/options.go index f21912687c2..086490879b7 100644 --- a/pkg/storage/utils/decomposedfs/options/options.go +++ b/pkg/storage/utils/decomposedfs/options/options.go @@ -73,9 +73,10 @@ type Options struct { Tokens TokenOptions `mapstructure:"tokens"` - StatCache cache.Config `mapstructure:"statcache"` + // FileMetadataCache for file metadata FileMetadataCache cache.Config `mapstructure:"filemetadatacache"` - IDCache cache.Config `mapstructure:"idcache"` + // IDCache for symlink lookups of direntry to node id + IDCache cache.Config `mapstructure:"idcache"` MaxAcquireLockCycles int `mapstructure:"max_acquire_lock_cycles"` LockCycleDurationFactor int `mapstructure:"lock_cycle_duration_factor"` diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 6fc5176af31..1a1dae32f90 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -70,6 +70,7 @@ type Tree struct { options *options.Options + // used to cache symlink lookups for child names to node ids idCache store.Store } From c853dcc3e4da9b73527df3d20a101fe82ea1b2c1 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 1 Mar 2024 16:11:03 +0100 Subject: [PATCH 03/16] add createcontainer to serviceaccount Signed-off-by: jkoberg --- changelog/unreleased/extend-service-account-permissions.md | 5 +++++ pkg/storage/utils/decomposedfs/node/permissions.go | 1 + 2 files changed, 6 insertions(+) create mode 100644 changelog/unreleased/extend-service-account-permissions.md diff --git a/changelog/unreleased/extend-service-account-permissions.md b/changelog/unreleased/extend-service-account-permissions.md new file mode 100644 index 00000000000..1c53c729d5b --- /dev/null +++ b/changelog/unreleased/extend-service-account-permissions.md @@ -0,0 +1,5 @@ +Enhancement: Extend service account permissions + +Adds CreateContainer permisson and improves cs3 storage pkg + +https://github.com/cs3org/reva/pull/4543 diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index 1367a6d5d89..4e3459a9467 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -99,6 +99,7 @@ func ServiceAccountPermissions() provider.ResourcePermissions { PurgeRecycle: true, // for purge-trash-bin command RestoreRecycleItem: true, // for cli restore command Delete: true, // for cli restore command with replace option + CreateContainer: true, // for space provisioning } } From b94fe40d99ff275b0add4a352cd8cd0904a3d304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 5 Mar 2024 12:35:30 +0100 Subject: [PATCH 04/16] give util functions a context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/context-for-utils.md | 5 +++++ pkg/utils/grpc.go | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/context-for-utils.md diff --git a/changelog/unreleased/context-for-utils.md b/changelog/unreleased/context-for-utils.md new file mode 100644 index 00000000000..299b0d00497 --- /dev/null +++ b/changelog/unreleased/context-for-utils.md @@ -0,0 +1,5 @@ +Enhancement: Allow tracing requests by giving util functions a context + +We deprecated GetServiceUserContext with GetServiceUserContextWithContext and GetUser with GetUserWithContext to allow passing in a trace context. + +https://github.com/cs3org/reva/pull/4556 \ No newline at end of file diff --git a/pkg/utils/grpc.go b/pkg/utils/grpc.go index ae49c7ab8e2..d363d9f2da6 100644 --- a/pkg/utils/grpc.go +++ b/pkg/utils/grpc.go @@ -38,8 +38,13 @@ var ( ) // GetServiceUserContext returns an authenticated context of the given service user +// Deprecated: Use GetServiceUserContextWithContext() func GetServiceUserContext(serviceUserID string, gwc gateway.GatewayAPIClient, serviceUserSecret string) (context.Context, error) { - ctx := context.Background() + return GetServiceUserContextWithContext(context.Background(), gwc, serviceUserID, serviceUserSecret) +} + +// GetServiceUserContextWithContext returns an authenticated context of the given service user +func GetServiceUserContextWithContext(ctx context.Context, gwc gateway.GatewayAPIClient, serviceUserID string, serviceUserSecret string) (context.Context, error) { authRes, err := gwc.Authenticate(ctx, &gateway.AuthenticateRequest{ Type: "serviceaccounts", ClientId: serviceUserID, @@ -57,8 +62,14 @@ func GetServiceUserContext(serviceUserID string, gwc gateway.GatewayAPIClient, s } // GetUser gets the specified user +// Deprecated: Use GetUserWithContext() func GetUser(userID *user.UserId, gwc gateway.GatewayAPIClient) (*user.User, error) { - getUserResponse, err := gwc.GetUser(context.Background(), &user.GetUserRequest{UserId: userID}) + return GetUserWithContext(context.Background(), userID, gwc) +} + +// GetUserWithContext gets the specified user +func GetUserWithContext(ctx context.Context, userID *user.UserId, gwc gateway.GatewayAPIClient) (*user.User, error) { + getUserResponse, err := gwc.GetUser(ctx, &user.GetUserRequest{UserId: userID}) if err != nil { return nil, err } From 94f7dec293a280547705cb3079c49ddb15faaf89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 5 Mar 2024 13:03:26 +0100 Subject: [PATCH 05/16] fix ceph build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- Dockerfile.revad | 2 +- Dockerfile.revad-ceph | 22 ++++++++++++++++------ changelog/unreleased/fix-ceph-build.md | 3 +++ 3 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/fix-ceph-build.md diff --git a/Dockerfile.revad b/Dockerfile.revad index 5bca9ef4259..b83c3a135c9 100644 --- a/Dockerfile.revad +++ b/Dockerfile.revad @@ -34,7 +34,7 @@ COPY . . RUN make build-revad-docker && \ cp /go/src/github/cs3org/reva/cmd/revad/revad /go/bin/revad -RUN mkdir -p /etc/revad/ && echo "" > /etc/revad/revad.toml +RUN mkdir -p /etc/revad/ && touch /etc/revad/revad.toml FROM golang:1.21-alpine3.18 diff --git a/Dockerfile.revad-ceph b/Dockerfile.revad-ceph index 9e8f8d36f60..39699332bdc 100644 --- a/Dockerfile.revad-ceph +++ b/Dockerfile.revad-ceph @@ -16,9 +16,14 @@ # granted to it by virtue of its status as an Intergovernmental Organization # or submit itself to any jurisdiction. -FROM quay.io/ceph/ceph:v16 +FROM quay.io/ceph/ceph:v18 -RUN dnf update --exclude=ceph-iscsi -y && dnf install -y \ +# replace repo url with one that allows downloading the repo metadata +# if http://download.ceph.com/rpm-reef/el8/x86_64/repodata/repomd.xml works again this can be dropped +RUN sed -i 's/download.ceph.com/de.ceph.com/' /etc/yum.repos.d/ceph.repo +RUN mkdir -p /etc/selinux/config + +RUN dnf update --exclude=ceph-iscsi,chrony -y && dnf install -y \ git \ gcc \ make \ @@ -26,7 +31,7 @@ RUN dnf update --exclude=ceph-iscsi -y && dnf install -y \ librbd-devel \ librados-devel -ADD https://golang.org/dl/go1.21.5.linux-amd64.tar.gz \ +ADD https://go.dev/dl/go1.21.5.linux-amd64.tar.gz \ go1.21.5.linux-amd64.tar.gz RUN rm -rf /usr/local/go && \ @@ -38,13 +43,18 @@ ENV GOPATH /go WORKDIR /go/src/github/cs3org/reva COPY . . + +ARG GIT_COMMIT +ARG VERSION +ENV GIT_COMMIT=$GIT_COMMIT +ENV VERSION=$VERSION RUN mkdir -p /go/bin && \ - make build-revad-cephfs-docker && \ - cp /go/src/github/cs3org/reva/cmd/revad/revad /usr/bin/revad + make build-revad-cephfs-docker && \ + cp /go/src/github/cs3org/reva/cmd/revad/revad /usr/bin/revad RUN cp -r examples/ceph /etc/ -RUN mkdir -p /etc/revad/ && echo "" > /etc/revad/revad.toml +RUN mkdir -p /etc/revad/ && touch /etc/revad/revad.toml EXPOSE 9999 10000 diff --git a/changelog/unreleased/fix-ceph-build.md b/changelog/unreleased/fix-ceph-build.md new file mode 100644 index 00000000000..0e03afbef91 --- /dev/null +++ b/changelog/unreleased/fix-ceph-build.md @@ -0,0 +1,3 @@ +Bugfix: Fix ceph build + +https://github.com/cs3org/reva/pull/4557 From 0a20b86309660151a051612ccfe1a24675422589 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 1 Mar 2024 16:24:33 +0100 Subject: [PATCH 06/16] allow cs3 storage to be configured manually Signed-off-by: jkoberg --- .../extend-service-account-permissions.md | 2 +- pkg/storage/utils/metadata/cs3.go | 44 +++++++++++++------ pkg/storage/utils/metadata/storage.go | 3 +- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/changelog/unreleased/extend-service-account-permissions.md b/changelog/unreleased/extend-service-account-permissions.md index 1c53c729d5b..2e063675ca5 100644 --- a/changelog/unreleased/extend-service-account-permissions.md +++ b/changelog/unreleased/extend-service-account-permissions.md @@ -2,4 +2,4 @@ Enhancement: Extend service account permissions Adds CreateContainer permisson and improves cs3 storage pkg -https://github.com/cs3org/reva/pull/4543 +https://github.com/cs3org/reva/pull/4545 diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index c8ee7b9fb65..c68ce114fb6 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -51,30 +51,41 @@ func init() { // CS3 represents a metadata storage with a cs3 storage backend type CS3 struct { + SpaceRoot *provider.ResourceId + providerAddr string gatewayAddr string + useSystemUser bool serviceUser *user.User machineAuthAPIKey string + dataGatewayClient *http.Client - SpaceRoot *provider.ResourceId } -// NewCS3Storage returns a new cs3 storage instance -func NewCS3Storage(gwAddr, providerAddr, serviceUserID, serviceUserIDP, machineAuthAPIKey string) (s Storage, err error) { - c := http.DefaultClient - +// NewCS3 returns a new CS3 instance. Use an authenticated context and be sure to define SpaceRoot manually. +func NewCS3(gwAddr, providerAddr string) (s *CS3) { return &CS3{ providerAddr: providerAddr, gatewayAddr: gwAddr, - dataGatewayClient: c, - machineAuthAPIKey: machineAuthAPIKey, - serviceUser: &user.User{ - Id: &user.UserId{ - OpaqueId: serviceUserID, - Idp: serviceUserIDP, - }, + dataGatewayClient: http.DefaultClient, + } +} + +// NewCS3Storage returns a new cs3 storage instance. Context passed to methods is irrelevant as the service user will be used. +// Be sure to call Init before using the storage. +func NewCS3Storage(gwAddr, providerAddr, serviceUserID, serviceUserIDP, machineAuthAPIKey string) (s Storage, err error) { + cs3 := NewCS3(gwAddr, providerAddr) + + cs3.useSystemUser = true + cs3.machineAuthAPIKey = machineAuthAPIKey + cs3.serviceUser = &user.User{ + Id: &user.UserId{ + OpaqueId: serviceUserID, + Idp: serviceUserIDP, }, - }, nil + } + + return cs3, nil } // Backend returns the backend name of the storage @@ -228,7 +239,8 @@ func (cs3 *CS3) Upload(ctx context.Context, req UploadRequest) (*UploadResponse, etag = ocEtag } return &UploadResponse{ - Etag: etag, + Etag: etag, + FileID: resp.Header.Get("OC-Fileid"), }, nil } @@ -505,6 +517,10 @@ func (cs3 *CS3) providerClient() (provider.ProviderAPIClient, error) { } func (cs3 *CS3) getAuthContext(ctx context.Context) (context.Context, error) { + if !cs3.useSystemUser { + return ctx, nil + } + // we need to start a new context to get rid of an existing x-access-token in the outgoing context authCtx := context.Background() authCtx, span := tracer.Start(authCtx, "getAuthContext", trace.WithLinks(trace.LinkFromContext(ctx))) diff --git a/pkg/storage/utils/metadata/storage.go b/pkg/storage/utils/metadata/storage.go index 7c6107cff0a..24e74f4a9f0 100644 --- a/pkg/storage/utils/metadata/storage.go +++ b/pkg/storage/utils/metadata/storage.go @@ -43,7 +43,8 @@ type UploadRequest struct { // UploadResponse represents a upload response type UploadResponse struct { - Etag string + Etag string + FileID string // only for cs3 storage } // DownloadRequest represents a download request and its options From afc0755b5f7dc7b204430c5f6d5574b6bac01fd1 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 5 Mar 2024 18:41:43 +0100 Subject: [PATCH 07/16] disallow to share the personal drive --- changelog/unreleased/fix-graph-invite.md | 6 ++++++ internal/grpc/services/gateway/gateway.go | 5 +++++ internal/grpc/services/gateway/usershareprovider.go | 4 ++++ 3 files changed, 15 insertions(+) create mode 100644 changelog/unreleased/fix-graph-invite.md diff --git a/changelog/unreleased/fix-graph-invite.md b/changelog/unreleased/fix-graph-invite.md new file mode 100644 index 00000000000..29169dd3443 --- /dev/null +++ b/changelog/unreleased/fix-graph-invite.md @@ -0,0 +1,6 @@ +Bugfix: Fix graph drive invite + +We fixed the issue when sharing of personal drive is allowed via graph + +https://github.com/cs3org/reva/pull/4559 +https://github.com/owncloud/ocis/issues/8494 diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go index b8e6533da96..23bc84274a7 100644 --- a/internal/grpc/services/gateway/gateway.go +++ b/internal/grpc/services/gateway/gateway.go @@ -35,6 +35,11 @@ import ( "google.golang.org/grpc" ) +const ( + _spaceTypePersonal = "personal" + _spaceTypeProject = "project" +) + func init() { rgrpc.Register("gateway", New) } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 9a27f8f9c55..ebfd5241dc4 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -585,6 +585,10 @@ func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareReques } func (s *svc) addSpaceShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { + if refIsSpaceRoot(req.GetResourceInfo().GetId()) && + req.GetResourceInfo().GetSpace().GetSpaceType() == _spaceTypePersonal { + return nil, errors.New("gateway: space type is not eligible for sharing") + } // If the share is a denial we call denyGrant instead. var st *rpc.Status var err error From 698d86209c3dd32f6fd838b15793e118269f005d Mon Sep 17 00:00:00 2001 From: Prarup Gurung Date: Thu, 7 Mar 2024 12:23:56 +0545 Subject: [PATCH 08/16] Bump ocis commit id to latest (#4563) --- .drone.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.env b/.drone.env index 81f9c725e2e..e0bb41076a6 100644 --- a/.drone.env +++ b/.drone.env @@ -1,4 +1,4 @@ # The test runner source for API tests -APITESTS_COMMITID=d33935c25756878c10e28b28240aade7e2b79da0 +APITESTS_COMMITID=2e7f0252f2a1b2c8200bf0931274b9e9e83f3c0a APITESTS_BRANCH=master APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git From 9da2e1831738a6c1e0c886c771a4ed7f997ff573 Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Thu, 7 Mar 2024 10:09:41 +0100 Subject: [PATCH 09/16] [full-ci] fix an error when lock/unlock a file (#4518) * [full-ci] fix an error when lock/unlock a public shared file * added errtype to http code mapping * reworked the error handling --------- Co-authored-by: Roman Perekhod --- changelog/unreleased/fix-public-link-lock.md | 6 ++ .../storageprovider/storageprovider.go | 24 ++++++ .../services/owncloud/ocdav/errors/error.go | 18 +++++ .../http/services/owncloud/ocdav/locks.go | 79 +++++++++---------- pkg/errtypes/errtypes.go | 43 +++++++++- 5 files changed, 124 insertions(+), 46 deletions(-) create mode 100644 changelog/unreleased/fix-public-link-lock.md diff --git a/changelog/unreleased/fix-public-link-lock.md b/changelog/unreleased/fix-public-link-lock.md new file mode 100644 index 00000000000..42a53432446 --- /dev/null +++ b/changelog/unreleased/fix-public-link-lock.md @@ -0,0 +1,6 @@ +Bugfix: Fix an error when lock/unlock a file + +We fixed a bug when anonymous user with viewer role in public link of a folder can lock/unlock a file inside it + +https://github.com/cs3org/reva/pull/4518 +https://github.com/owncloud/ocis/issues/7785 diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index c125698c0b9..2a9982d2e8c 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -33,6 +33,7 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" + "github.com/cs3org/reva/v2/pkg/conversions" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/events" @@ -230,6 +231,11 @@ func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.Unse // SetLock puts a lock on the given reference func (s *service) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provider.SetLockResponse, error) { + if !canLockPublicShare(ctx) { + return &provider.SetLockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to lock the share"), + }, nil + } err := s.storage.SetLock(ctx, req.Ref, req.Lock) return &provider.SetLockResponse{ @@ -249,6 +255,12 @@ func (s *service) GetLock(ctx context.Context, req *provider.GetLockRequest) (*p // RefreshLock refreshes an existing lock on the given reference func (s *service) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) (*provider.RefreshLockResponse, error) { + if !canLockPublicShare(ctx) { + return &provider.RefreshLockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to refresh the share lock"), + }, nil + } + err := s.storage.RefreshLock(ctx, req.Ref, req.Lock, req.ExistingLockId) return &provider.RefreshLockResponse{ @@ -258,6 +270,12 @@ func (s *service) RefreshLock(ctx context.Context, req *provider.RefreshLockRequ // Unlock removes an existing lock from the given reference func (s *service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provider.UnlockResponse, error) { + if !canLockPublicShare(ctx) { + return &provider.UnlockResponse{ + Status: status.NewPermissionDenied(ctx, nil, "no permission to unlock the share"), + }, nil + } + err := s.storage.Unlock(ctx, req.Ref, req.Lock) return &provider.UnlockResponse{ @@ -1285,3 +1303,9 @@ func estreamFromConfig(c eventconfig) (events.Stream, error) { return stream.NatsFromConfig("storageprovider", false, stream.NatsConfig(c)) } + +func canLockPublicShare(ctx context.Context) bool { + u := ctxpkg.ContextMustGetUser(ctx) + psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role") + return psr == "" || psr == conversions.RoleEditor +} diff --git a/internal/http/services/owncloud/ocdav/errors/error.go b/internal/http/services/owncloud/ocdav/errors/error.go index 78df1d19835..adf55e589c7 100644 --- a/internal/http/services/owncloud/ocdav/errors/error.go +++ b/internal/http/services/owncloud/ocdav/errors/error.go @@ -21,6 +21,7 @@ package errors import ( "bytes" "encoding/xml" + "fmt" "net/http" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -193,3 +194,20 @@ func HandleWebdavError(log *zerolog.Logger, w http.ResponseWriter, b []byte, err log.Err(err).Msg("error writing response") } } + +func NewErrFromStatus(s *rpc.Status) error { + switch s.GetCode() { + case rpc.Code_CODE_OK: + return nil + case rpc.Code_CODE_DEADLINE_EXCEEDED: + return ErrInvalidTimeout + case rpc.Code_CODE_PERMISSION_DENIED: + return ErrForbidden + case rpc.Code_CODE_LOCKED, rpc.Code_CODE_FAILED_PRECONDITION: + return ErrLocked + case rpc.Code_CODE_UNIMPLEMENTED: + return ErrNotImplemented + default: + return fmt.Errorf(s.GetMessage()) + } +} diff --git a/internal/http/services/owncloud/ocdav/locks.go b/internal/http/services/owncloud/ocdav/locks.go index 332b9d22760..6760bfd90d7 100644 --- a/internal/http/services/owncloud/ocdav/locks.go +++ b/internal/http/services/owncloud/ocdav/locks.go @@ -21,6 +21,7 @@ package ocdav import ( "context" "encoding/xml" + "errors" "fmt" "io" "net/http" @@ -34,13 +35,12 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors" + ocdavErrors "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/prop" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" - "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/utils" "github.com/google/uuid" @@ -172,7 +172,7 @@ type cs3LS struct { } func (cls *cs3LS) Confirm(ctx context.Context, now time.Time, name0, name1 string, conditions ...Condition) (func(), error) { - return nil, errors.ErrNotImplemented + return nil, ocdavErrors.ErrNotImplemented } func (cls *cs3LS) Create(ctx context.Context, now time.Time, details LockDetails) (string, error) { @@ -180,7 +180,7 @@ func (cls *cs3LS) Create(ctx context.Context, now time.Time, details LockDetails /* if !details.ZeroDepth { The CS3 Lock api currently has no depth property, it only locks single resources - return "", errors.ErrUnsupportedLockInfo + return "", ocdavErrors.ErrUnsupportedLockInfo } */ @@ -225,20 +225,19 @@ func (cls *cs3LS) Create(ctx context.Context, now time.Time, details LockDetails if err != nil { return "", err } - switch res.Status.Code { + switch res.GetStatus().GetCode() { case rpc.Code_CODE_OK: return lockTokenPrefix + token.String(), nil - case rpc.Code_CODE_FAILED_PRECONDITION: - return "", errtypes.Aborted("file is already locked") default: - return "", errtypes.NewErrtypeFromStatus(res.Status) + return "", ocdavErrors.NewErrFromStatus(res.GetStatus()) } } func (cls *cs3LS) Refresh(ctx context.Context, now time.Time, token string, duration time.Duration) (LockDetails, error) { - return LockDetails{}, errors.ErrNotImplemented + return LockDetails{}, ocdavErrors.ErrNotImplemented } + func (cls *cs3LS) Unlock(ctx context.Context, now time.Time, ref *provider.Reference, token string) error { u := ctxpkg.ContextMustGetUser(ctx) @@ -260,14 +259,11 @@ func (cls *cs3LS) Unlock(ctx context.Context, now time.Time, ref *provider.Refer return err } - switch res.Status.Code { - case rpc.Code_CODE_OK: - return nil - case rpc.Code_CODE_FAILED_PRECONDITION: - return errtypes.Aborted("file is not locked") - default: - return errtypes.NewErrtypeFromStatus(res.Status) + newErr := ocdavErrors.NewErrFromStatus(res.GetStatus()) + if newErr != nil { + appctx.GetLogger(ctx).Error().Str("token", token).Interface("unlock", ref).Msg("could not unlock " + res.GetStatus().GetMessage()) } + return newErr } // LockDetails are a lock's metadata. @@ -302,7 +298,7 @@ func readLockInfo(r io.Reader) (li lockInfo, status int, err error) { // http://www.webdav.org/specs/rfc4918.html#refreshing-locks return lockInfo{}, 0, nil } - err = errors.ErrInvalidLockInfo + err = ocdavErrors.ErrInvalidLockInfo } return lockInfo{}, http.StatusBadRequest, err } @@ -349,15 +345,15 @@ func parseTimeout(s string) (time.Duration, error) { } const pre = "Second-" if !strings.HasPrefix(s, pre) { - return 0, errors.ErrInvalidTimeout + return 0, ocdavErrors.ErrInvalidTimeout } s = s[len(pre):] if s == "" || s[0] < '0' || '9' < s[0] { - return 0, errors.ErrInvalidTimeout + return 0, ocdavErrors.ErrInvalidTimeout } n, err := strconv.ParseInt(s, 10, 64) if err != nil || 1<<32-1 < n { - return 0, errors.ErrInvalidTimeout + return 0, ocdavErrors.ErrInvalidTimeout } return time.Duration(n) * time.Second, nil } @@ -420,7 +416,7 @@ func (s *svc) handleLock(w http.ResponseWriter, r *http.Request, ns string) (ret return http.StatusInternalServerError, err } if cs3Status.Code != rpc.Code_CODE_OK { - return http.StatusInternalServerError, errtypes.NewErrtypeFromStatus(cs3Status) + return http.StatusInternalServerError, ocdavErrors.NewErrFromStatus(cs3Status) } return s.lockReference(ctx, w, r, ref) @@ -444,12 +440,12 @@ func (s *svc) lockReference(ctx context.Context, w http.ResponseWriter, r *http. sublog := appctx.GetLogger(ctx).With().Interface("ref", ref).Logger() duration, err := parseTimeout(r.Header.Get(net.HeaderTimeout)) if err != nil { - return http.StatusBadRequest, errors.ErrInvalidTimeout + return http.StatusBadRequest, ocdavErrors.ErrInvalidTimeout } li, status, err := readLockInfo(r.Body) if err != nil { - return status, errors.ErrInvalidLockInfo + return status, ocdavErrors.ErrInvalidLockInfo } u := ctxpkg.ContextMustGetUser(ctx) @@ -459,17 +455,17 @@ func (s *svc) lockReference(ctx context.Context, w http.ResponseWriter, r *http. // An empty lockInfo means to refresh the lock. ih, ok := parseIfHeader(r.Header.Get(net.HeaderIf)) if !ok { - return http.StatusBadRequest, errors.ErrInvalidIfHeader + return http.StatusBadRequest, ocdavErrors.ErrInvalidIfHeader } if len(ih.lists) == 1 && len(ih.lists[0].conditions) == 1 { token = ih.lists[0].conditions[0].Token } if token == "" { - return http.StatusBadRequest, errors.ErrInvalidLockToken + return http.StatusBadRequest, ocdavErrors.ErrInvalidLockToken } ld, err = s.LockSystem.Refresh(ctx, now, token, duration) if err != nil { - if err == errors.ErrNoSuchLock { + if err == ocdavErrors.ErrNoSuchLock { return http.StatusPreconditionFailed, err } return http.StatusInternalServerError, err @@ -484,7 +480,7 @@ func (s *svc) lockReference(ctx context.Context, w http.ResponseWriter, r *http. if depth != 0 && depth != infiniteDepth { // Section 9.10.3 says that "Values other than 0 or infinity must not be // used with the Depth header on a LOCK method". - return http.StatusBadRequest, errors.ErrInvalidDepth + return http.StatusBadRequest, ocdavErrors.ErrInvalidDepth } } /* our url path has been shifted, so we don't need to do this? @@ -505,15 +501,16 @@ func (s *svc) lockReference(ctx context.Context, w http.ResponseWriter, r *http. // should we do that in the decomposedfs as well? the node does not exist // this actually is a name based lock ... ugh token, err = s.LockSystem.Create(ctx, now, ld) + + // if err != nil { - switch err.(type) { - case errtypes.Aborted: + switch { + case errors.Is(err, ocdavErrors.ErrLocked): return http.StatusLocked, err - case errtypes.PermissionDenied: + case errors.Is(err, ocdavErrors.ErrForbidden): return http.StatusForbidden, err default: return http.StatusInternalServerError, err - } } @@ -611,7 +608,7 @@ func (s *svc) handleUnlock(w http.ResponseWriter, r *http.Request, ns string) (s return http.StatusInternalServerError, err } if cs3Status.Code != rpc.Code_CODE_OK { - return http.StatusInternalServerError, errtypes.NewErrtypeFromStatus(cs3Status) + return http.StatusInternalServerError, ocdavErrors.NewErrFromStatus(cs3Status) } return s.unlockReference(ctx, w, r, ref) @@ -639,16 +636,14 @@ func (s *svc) unlockReference(ctx context.Context, _ http.ResponseWriter, r *htt t = t[1 : len(t)-1] } - switch err := s.LockSystem.Unlock(ctx, time.Now(), ref, t); err { - case nil: - return http.StatusNoContent, err - case errors.ErrForbidden: - return http.StatusForbidden, err - case errors.ErrLocked: + err := s.LockSystem.Unlock(ctx, time.Now(), ref, t) + switch { + case err == nil: + return http.StatusNoContent, nil + case errors.Is(err, ocdavErrors.ErrLocked): return http.StatusLocked, err - case errors.ErrNoSuchLock: - return http.StatusConflict, err - default: - return http.StatusInternalServerError, err + case errors.Is(err, ocdavErrors.ErrForbidden): + return http.StatusForbidden, err } + return http.StatusInternalServerError, err } diff --git a/pkg/errtypes/errtypes.go b/pkg/errtypes/errtypes.go index 4aff2129312..07266f1184c 100644 --- a/pkg/errtypes/errtypes.go +++ b/pkg/errtypes/errtypes.go @@ -295,12 +295,13 @@ func NewErrtypeFromStatus(status *rpc.Status) error { case rpc.Code_CODE_UNIMPLEMENTED: return NotSupported(status.Message) case rpc.Code_CODE_PERMISSION_DENIED: - // FIXME add locked status! - if strings.HasPrefix(status.Message, "set lock: error: locked by ") { - return Locked(strings.TrimPrefix(status.Message, "set lock: error: locked by ")) - } return PermissionDenied(status.Message) case rpc.Code_CODE_LOCKED: + // FIXME make something better for that + msg := strings.Split(status.Message, "error: locked by ") + if len(msg) > 1 { + return Locked(msg[len(msg)-1]) + } return Locked(status.Message) // case rpc.Code_CODE_DATA_LOSS: ? // IsPartialContent @@ -350,3 +351,37 @@ func NewErrtypeFromHTTPStatusCode(code int, message string) error { return InternalError(message) } } + +// NewHTTPStatusCodeFromErrtype maps an errtype to an http status +func NewHTTPStatusCodeFromErrtype(err error) int { + switch err.(type) { + case NotFound: + return http.StatusNotFound + case AlreadyExists: + return http.StatusConflict + case NotSupported: + return http.StatusNotImplemented + case NotModified: + return http.StatusNotModified + case InvalidCredentials: + return http.StatusUnauthorized + case PermissionDenied: + return http.StatusForbidden + case Locked: + return http.StatusLocked + case Aborted: + return http.StatusPreconditionFailed + case PreconditionFailed: + return http.StatusMethodNotAllowed + case InsufficientStorage: + return http.StatusInsufficientStorage + case BadRequest: + return http.StatusBadRequest + case PartialContent: + return http.StatusPartialContent + case ChecksumMismatch: + return StatusChecksumMismatch + default: + return http.StatusInternalServerError + } +} From 5d70357fda99103f7ba11a6da2c32f10bec9afb4 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 6 Mar 2024 11:08:40 +0100 Subject: [PATCH 10/16] fix(sharestorageprovider): Remove misleading comment --- .../services/sharesstorageprovider/sharesstorageprovider.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 8d5316c6d93..f2e738f4bc5 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -1060,8 +1060,6 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere // we currently need to list all shares and match the path if the request is relative to the share jail root if ref.ResourceId.OpaqueId == utils.ShareStorageProviderID && ref.Path != "." { // we need to list accepted shares and match the path - - // look up share for this resourceid lsRes, err := sharingCollaborationClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ Filters: []*collaboration.Filter{ { From 24bab56f29e571d93f6409261d5abcd0d9d13d77 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 6 Mar 2024 11:11:33 +0100 Subject: [PATCH 11/16] fix(sharesstorageprovider): Remove unneeded code We're already using a State based filter in the ListReceivedShares request. No need to manually check for that again. --- .../services/sharesstorageprovider/sharesstorageprovider.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index f2e738f4bc5..7c5336a7a1a 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -1078,10 +1078,6 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere return nil, lsRes.Status, nil } for _, receivedShare := range lsRes.Shares { - // make sure to skip unaccepted shares - if receivedShare.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { - continue - } if isMountPointForPath(receivedShare.MountPoint.Path, ref.Path) { return receivedShare, lsRes.Status, nil } From fa2caba1f4e488f7e4c0c9d7b215969d14a94d6a Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 6 Mar 2024 13:05:40 +0100 Subject: [PATCH 12/16] fix(sharestorageprovider): Stat resource when resolving share by path When resolving a received shared by mountpath, make sure that the shared resource actually exists. This avoids issues with dangling shares for already delete resources using the same mountpoint. Fixes: https://github.com/owncloud/ocis/issues/7895 --- .../unreleased/fix-stat-recreated-share.md | 8 +++++ .../sharesstorageprovider.go | 29 +++++++++++++++++-- .../sharesstorageprovider_test.go | 25 +++++++++++++--- 3 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/fix-stat-recreated-share.md diff --git a/changelog/unreleased/fix-stat-recreated-share.md b/changelog/unreleased/fix-stat-recreated-share.md new file mode 100644 index 00000000000..480cd4cf88d --- /dev/null +++ b/changelog/unreleased/fix-stat-recreated-share.md @@ -0,0 +1,8 @@ +Bugfix: Fix Stat() by Path on re-created resource + +We fixed bug that caused Stat Requests using a Path reference to a mount point +in the sharejail to not resolve correctly, when a share using the same +mount point to an already deleted resource was still existing. + +https://github.com/cs3org/reva/pull/4561 +https://github.com/owncloud/ocis/issues/7895 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 7c5336a7a1a..5fd8db144c3 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -1057,9 +1057,11 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere return lsRes.Share, lsRes.Status, nil } - // we currently need to list all shares and match the path if the request is relative to the share jail root + // we currently need to list all accepted shares and match the path if the + // request is relative to the share jail root. Also we need to Stat() the + // shared resource's id to check whether that still exist. There might be + // old shares using the same path but for an already vanished resource id. if ref.ResourceId.OpaqueId == utils.ShareStorageProviderID && ref.Path != "." { - // we need to list accepted shares and match the path lsRes, err := sharingCollaborationClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ Filters: []*collaboration.Filter{ { @@ -1079,10 +1081,31 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere } for _, receivedShare := range lsRes.Shares { if isMountPointForPath(receivedShare.MountPoint.Path, ref.Path) { + // Only return this share if the resource still exists. + gatewayClient, err := s.gatewaySelector.Next() + if err != nil { + return nil, nil, err + } + sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ResourceId: receivedShare.GetShare().GetResourceId()}, + }) + if err != nil { + appctx.GetLogger(ctx).Debug(). + Err(err). + Interface("resourceID", receivedShare.GetShare().GetResourceId()). + Msg("resolveAcceptedShare: failed to stat shared resource") + continue + } + if sRes.Status.Code != rpc.Code_CODE_OK { + appctx.GetLogger(ctx).Debug(). + Interface("resourceID", receivedShare.GetShare().GetResourceId()). + Interface("status", sRes.Status). + Msg("resolveAcceptedShare: failed to stat shared resource") + continue + } return receivedShare, lsRes.Status, nil } } - } return nil, status.NewNotFound(ctx, "sharesstorageprovider: not found "+ref.String()), nil diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 9ff1f8592a4..d2921a53e6e 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -261,7 +261,23 @@ var _ = Describe("Sharesstorageprovider", func() { }, } default: - if req.Ref.ResourceId.OpaqueId == "shareddir-merged" { + switch req.Ref.ResourceId.OpaqueId { + case "shareddir", "shareddir2": + permissionSet := &sprovider.ResourcePermissions{ + Stat: true, + ListContainer: true, + } + return &sprovider.StatResponse{ + Status: status.NewOK(context.Background()), + Info: &sprovider.ResourceInfo{ + Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER, + Path: "share1-shareddir", + Id: req.Ref.ResourceId, + PermissionSet: permissionSet, + Size: 100, + }, + } + case "shareddir-merged": permissionSet := &sprovider.ResourcePermissions{ Stat: true, ListContainer: true, @@ -280,9 +296,10 @@ var _ = Describe("Sharesstorageprovider", func() { Size: 100, }, } - } - return &sprovider.StatResponse{ - Status: status.NewNotFound(context.Background(), "not found"), + default: + return &sprovider.StatResponse{ + Status: status.NewNotFound(context.Background(), "not found"), + } } } }, From 22fb438c48dd21ecc671273c37c4769fa5f47d16 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Thu, 7 Mar 2024 13:38:22 +0100 Subject: [PATCH 13/16] send fileunlocked events Signed-off-by: jkoberg --- changelog/unreleased/file-unlocked-events.md | 5 ++++ .../eventsmiddleware/conversion.go | 18 +++++++++++ .../interceptors/eventsmiddleware/events.go | 9 ++++++ pkg/events/files.go | 30 +++++++++++++++++++ 4 files changed, 62 insertions(+) create mode 100644 changelog/unreleased/file-unlocked-events.md diff --git a/changelog/unreleased/file-unlocked-events.md b/changelog/unreleased/file-unlocked-events.md new file mode 100644 index 00000000000..2d82a494a0d --- /dev/null +++ b/changelog/unreleased/file-unlocked-events.md @@ -0,0 +1,5 @@ +Enhancement: Send file locked/unlocked events + +Emit an event when a file is locked or unlocked + +https://github.com/cs3org/reva/pull/4564 diff --git a/internal/grpc/interceptors/eventsmiddleware/conversion.go b/internal/grpc/interceptors/eventsmiddleware/conversion.go index a4ff32f62b1..edcab27e37b 100644 --- a/internal/grpc/interceptors/eventsmiddleware/conversion.go +++ b/internal/grpc/interceptors/eventsmiddleware/conversion.go @@ -216,6 +216,24 @@ func FileDownloaded(r *provider.InitiateFileDownloadResponse, req *provider.Init } } +// FileLocked converts the response to an events +func FileLocked(r *provider.SetLockResponse, req *provider.SetLockRequest, owner, executant *user.UserId) events.FileLocked { + return events.FileLocked{ + Executant: executant, + Ref: req.Ref, + Timestamp: utils.TSNow(), + } +} + +// FileUnlocked converts the response to an event +func FileUnlocked(r *provider.UnlockResponse, req *provider.UnlockRequest, owner, executant *user.UserId) events.FileUnlocked { + return events.FileUnlocked{ + Executant: executant, + Ref: req.Ref, + Timestamp: utils.TSNow(), + } +} + // ItemTrashed converts the response to an event func ItemTrashed(r *provider.DeleteResponse, req *provider.DeleteRequest, spaceOwner, executant *user.UserId) events.ItemTrashed { opaqueID := utils.ReadPlainFromOpaque(r.Opaque, "opaque_id") diff --git a/internal/grpc/interceptors/eventsmiddleware/events.go b/internal/grpc/interceptors/eventsmiddleware/events.go index 8566f8bdefa..558a96010d2 100644 --- a/internal/grpc/interceptors/eventsmiddleware/events.go +++ b/internal/grpc/interceptors/eventsmiddleware/events.go @@ -184,6 +184,15 @@ func NewUnary(m map[string]interface{}) (grpc.UnaryServerInterceptor, int, error if isSuccess(v) { ev = FileTouched(v, req.(*provider.TouchFileRequest), ownerID, executantID) } + case *provider.SetLockResponse: + fmt.Println("set lock response", v) + if isSuccess(v) { + ev = FileLocked(v, req.(*provider.SetLockRequest), ownerID, executantID) + } + case *provider.UnlockResponse: + if isSuccess(v) { + ev = FileUnlocked(v, req.(*provider.UnlockRequest), ownerID, executantID) + } } if ev != nil { diff --git a/pkg/events/files.go b/pkg/events/files.go index cc21a4b07d1..e4b1017fd23 100644 --- a/pkg/events/files.go +++ b/pkg/events/files.go @@ -88,6 +88,36 @@ func (FileDownloaded) Unmarshal(v []byte) (interface{}, error) { return e, err } +// FileLocked is emitted when a file is locked +type FileLocked struct { + Executant *user.UserId + Ref *provider.Reference + Owner *user.UserId + Timestamp *types.Timestamp +} + +// Unmarshal to fulfill umarshaller interface +func (FileLocked) Unmarshal(v []byte) (interface{}, error) { + e := FileLocked{} + err := json.Unmarshal(v, &e) + return e, err +} + +// FileUnlocked is emitted when a file is unlocked +type FileUnlocked struct { + Executant *user.UserId + Ref *provider.Reference + Owner *user.UserId + Timestamp *types.Timestamp +} + +// Unmarshal to fulfill umarshaller interface +func (FileUnlocked) Unmarshal(v []byte) (interface{}, error) { + e := FileUnlocked{} + err := json.Unmarshal(v, &e) + return e, err +} + // ItemTrashed is emitted when a file or folder is trashed type ItemTrashed struct { SpaceOwner *user.UserId From b82840f82c335dc2dfebc5e361e0afb90ed40611 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Fri, 8 Mar 2024 13:07:16 +0100 Subject: [PATCH 14/16] Fix: public link previews Signed-off-by: jkoberg --- changelog/unreleased/fix-publiclink-previews.md | 5 +++++ internal/http/services/owncloud/ocdav/dav.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/fix-publiclink-previews.md diff --git a/changelog/unreleased/fix-publiclink-previews.md b/changelog/unreleased/fix-publiclink-previews.md new file mode 100644 index 00000000000..a51f3ef5b8b --- /dev/null +++ b/changelog/unreleased/fix-publiclink-previews.md @@ -0,0 +1,5 @@ +Bugfix: Fix public link previews + +Fixes previews for public links + +https://github.com/cs3org/reva/pull/4566 diff --git a/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go index db26d729bd9..ec3fc85a3fe 100644 --- a/internal/http/services/owncloud/ocdav/dav.go +++ b/internal/http/services/owncloud/ocdav/dav.go @@ -255,7 +255,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler { sig := q.Get("signature") expiration := q.Get("expiration") // We restrict the pre-signed urls to downloads. - if sig != "" && expiration != "" && r.Method != http.MethodGet { + if sig != "" && expiration != "" && !(r.Method == http.MethodGet || r.Method == http.MethodHead) { w.WriteHeader(http.StatusUnauthorized) return } From 4cbe0af6fa74cb6d927332b7e60945874128cf20 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 12 Mar 2024 12:51:14 +0100 Subject: [PATCH 15/16] fix sharing invite on virtual drive --- changelog/unreleased/fix-graph-invite-virtual.md | 6 ++++++ internal/grpc/services/gateway/gateway.go | 1 + internal/grpc/services/gateway/usershareprovider.go | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/fix-graph-invite-virtual.md diff --git a/changelog/unreleased/fix-graph-invite-virtual.md b/changelog/unreleased/fix-graph-invite-virtual.md new file mode 100644 index 00000000000..1849e5c2fc6 --- /dev/null +++ b/changelog/unreleased/fix-graph-invite-virtual.md @@ -0,0 +1,6 @@ +Bugfix: Fix sharing invite on virtual drive + +We fixed the issue when sharing of virtual drive with other users was allowed + +https://github.com/cs3org/reva/pull/4568 +https://github.com/owncloud/ocis/issues/8495 diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go index c3a30e12ca5..55a5ea8a7a5 100644 --- a/internal/grpc/services/gateway/gateway.go +++ b/internal/grpc/services/gateway/gateway.go @@ -38,6 +38,7 @@ import ( const ( _spaceTypePersonal = "personal" _spaceTypeProject = "project" + _spaceTypeVirtual = "virtual" ) func init() { diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 910375d4704..02587d2f99c 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -583,8 +583,8 @@ func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareReques func (s *svc) addSpaceShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { if refIsSpaceRoot(req.GetResourceInfo().GetId()) && - req.GetResourceInfo().GetSpace().GetSpaceType() == _spaceTypePersonal { - return nil, errors.New("gateway: space type is not eligible for sharing") + (req.GetResourceInfo().GetSpace().GetSpaceType() == _spaceTypePersonal || req.GetResourceInfo().GetSpace().GetSpaceType() == _spaceTypeVirtual) { + return &collaboration.CreateShareResponse{Status: status.NewInvalid(ctx, "space type is not eligible for sharing")}, nil } // If the share is a denial we call denyGrant instead. var st *rpc.Status From 04c0a3136ac9ba8323b6e232c77ae34dfbf8249c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 8 Mar 2024 17:59:15 +0100 Subject: [PATCH 16/16] fix: authprovider error message when a user is not found in the auth.Manager --- .../fix-authprovider-error-message.md | 3 ++ .../services/authprovider/authprovider.go | 30 ++++++------------- pkg/errtypes/errtypes.go | 14 ++++----- pkg/rgrpc/status/status.go | 4 +++ 4 files changed, 23 insertions(+), 28 deletions(-) create mode 100644 changelog/unreleased/fix-authprovider-error-message.md diff --git a/changelog/unreleased/fix-authprovider-error-message.md b/changelog/unreleased/fix-authprovider-error-message.md new file mode 100644 index 00000000000..45eddbc4445 --- /dev/null +++ b/changelog/unreleased/fix-authprovider-error-message.md @@ -0,0 +1,3 @@ +Bugfix: Fix error message in authprovider if user is not found + +https://github.com/cs3org/reva/pull/4567 diff --git a/internal/grpc/services/authprovider/authprovider.go b/internal/grpc/services/authprovider/authprovider.go index 29acbbe629b..ec2ca9dc66b 100644 --- a/internal/grpc/services/authprovider/authprovider.go +++ b/internal/grpc/services/authprovider/authprovider.go @@ -136,28 +136,16 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe password := req.ClientSecret u, scope, err := s.authmgr.Authenticate(ctx, username, password) - switch v := err.(type) { - case nil: - log.Info().Msgf("user %s authenticated", u.Id) - return &provider.AuthenticateResponse{ - Status: status.NewOK(ctx), - User: u, - TokenScope: scope, - }, nil - case errtypes.InvalidCredentials: - return &provider.AuthenticateResponse{ - Status: status.NewPermissionDenied(ctx, v, "wrong password"), - }, nil - case errtypes.NotFound: - log.Debug().Str("client_id", username).Msg("unknown client id") - return &provider.AuthenticateResponse{ - Status: status.NewNotFound(ctx, "unknown client id"), - }, nil - default: - err = errors.Wrap(err, "authsvc: error in Authenticate") + if err != nil { + log.Debug().Str("client_id", username).Err(err).Msg("authsvc: error in Authenticate") return &provider.AuthenticateResponse{ - Status: status.NewUnauthenticated(ctx, err, "error authenticating user"), + Status: status.NewStatusFromErrType(ctx, "authsvc: error in Authenticate", err), }, nil } - + log.Info().Msgf("user %s authenticated", u.Id) + return &provider.AuthenticateResponse{ + Status: status.NewOK(ctx), + User: u, + TokenScope: scope, + }, nil } diff --git a/pkg/errtypes/errtypes.go b/pkg/errtypes/errtypes.go index 07266f1184c..363211ac5af 100644 --- a/pkg/errtypes/errtypes.go +++ b/pkg/errtypes/errtypes.go @@ -144,7 +144,7 @@ func (e BadRequest) Error() string { return "error: bad request: " + string(e) } // IsBadRequest implements the IsBadRequest interface. func (e BadRequest) IsBadRequest() {} -// ChecksumMismatch is the error to use when the sent hash does not match the calculated hash. +// ChecksumMismatch is the error to use when the transmitted hash does not match the calculated hash. type ChecksumMismatch string func (e ChecksumMismatch) Error() string { return "error: checksum mismatch: " + string(e) } @@ -178,7 +178,7 @@ type NotModified string func (e NotModified) Error() string { return "error: not modified: " + string(e) } -// NotModified implements the IsNotModified interface. +// IsNotModified implements the IsNotModified interface. func (e NotModified) IsNotModified() {} // StatusCode returns StatusInsufficientStorage, this implementation is needed to allow TUS to cast the correct http errors. @@ -196,7 +196,7 @@ func (e InsufficientStorage) Body() []byte { const StatusInsufficientStorage = 507 // IsNotFound is the interface to implement -// to specify that an a resource is not found. +// to specify that a resource is not found. type IsNotFound interface { IsNotFound() } @@ -238,7 +238,7 @@ type IsPermissionDenied interface { } // IsLocked is the interface to implement -// to specify that an resource is locked. +// to specify that a resource is locked. type IsLocked interface { IsLocked() } @@ -279,7 +279,7 @@ type IsInsufficientStorage interface { IsInsufficientStorage() } -// NewErrtypeFromStatus maps an rpc status to an errtype +// NewErrtypeFromStatus maps a rpc status to an errtype func NewErrtypeFromStatus(status *rpc.Status) error { switch status.Code { case rpc.Code_CODE_OK: @@ -318,7 +318,7 @@ func NewErrtypeFromStatus(status *rpc.Status) error { } } -// NewErrtypeFromHTTPStatusCode maps an http status to an errtype +// NewErrtypeFromHTTPStatusCode maps a http status to an errtype func NewErrtypeFromHTTPStatusCode(code int, message string) error { switch code { case http.StatusOK: @@ -352,7 +352,7 @@ func NewErrtypeFromHTTPStatusCode(code int, message string) error { } } -// NewHTTPStatusCodeFromErrtype maps an errtype to an http status +// NewHTTPStatusCodeFromErrtype maps an errtype to a http status func NewHTTPStatusCodeFromErrtype(err error) int { switch err.(type) { case NotFound: diff --git a/pkg/rgrpc/status/status.go b/pkg/rgrpc/status/status.go index 56b635a4b1e..4055cff4d99 100644 --- a/pkg/rgrpc/status/status.go +++ b/pkg/rgrpc/status/status.go @@ -166,10 +166,14 @@ func NewStatusFromErrType(ctx context.Context, msg string, err error) *rpc.Statu switch e := err.(type) { case nil: return NewOK(ctx) + case errtypes.NotFound: + return NewNotFound(ctx, msg+": "+err.Error()) case errtypes.IsNotFound: return NewNotFound(ctx, msg+": "+err.Error()) case errtypes.AlreadyExists: return NewAlreadyExists(ctx, err, msg+": "+err.Error()) + case errtypes.InvalidCredentials: + return NewPermissionDenied(ctx, e, msg+": "+err.Error()) case errtypes.IsInvalidCredentials: // TODO this maps badly return NewUnauthenticated(ctx, err, msg+": "+err.Error())