From d7538d897c34dccbbdfa36c53abba0d3a6012e38 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Thu, 27 Jan 2022 20:40:32 +0100 Subject: [PATCH 1/7] Reuse ocs role objects in other drivers --- changelog/unreleased/uniform-ocs-roles.md | 3 + cmd/reva/ocm-share-create.go | 22 +------ internal/grpc/interceptors/auth/scope.go | 46 ++++++++++++--- .../services/owncloud/ocs/conversions/role.go | 2 +- pkg/auth/manager/publicshares/publicshares.go | 6 +- pkg/auth/scope/scope.go | 2 +- pkg/cbox/utils/conversions.go | 41 ++----------- pkg/storage/utils/eosfs/eosfs.go | 57 +++---------------- 8 files changed, 63 insertions(+), 116 deletions(-) create mode 100644 changelog/unreleased/uniform-ocs-roles.md diff --git a/changelog/unreleased/uniform-ocs-roles.md b/changelog/unreleased/uniform-ocs-roles.md new file mode 100644 index 0000000000..9c953ecb13 --- /dev/null +++ b/changelog/unreleased/uniform-ocs-roles.md @@ -0,0 +1,3 @@ +Enhancement: Reuse ocs role objects in other drivers + +https://github.com/cs3org/reva/pull/2514 \ No newline at end of file diff --git a/cmd/reva/ocm-share-create.go b/cmd/reva/ocm-share-create.go index 06ce7c9105..10eb2d1691 100644 --- a/cmd/reva/ocm-share-create.go +++ b/cmd/reva/ocm-share-create.go @@ -31,6 +31,7 @@ import ( ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" "github.com/cs3org/reva/pkg/utils" "github.com/jedib0t/go-pretty/table" "github.com/pkg/errors" @@ -165,28 +166,11 @@ func ocmShareCreateCommand() *command { func getOCMSharePerm(p string) (*ocm.SharePermissions, int, error) { if p == viewerPermission { return &ocm.SharePermissions{ - Permissions: &provider.ResourcePermissions{ - GetPath: true, - InitiateFileDownload: true, - ListFileVersions: true, - ListContainer: true, - Stat: true, - }, + Permissions: conversions.NewViewerRole().CS3ResourcePermissions(), }, 1, nil } else if p == editorPermission { return &ocm.SharePermissions{ - Permissions: &provider.ResourcePermissions{ - GetPath: true, - InitiateFileDownload: true, - ListFileVersions: true, - ListContainer: true, - Stat: true, - CreateContainer: true, - Delete: true, - InitiateFileUpload: true, - RestoreFileVersion: true, - Move: true, - }, + Permissions: conversions.NewEditorRole().CS3ResourcePermissions(), }, 15, nil } return nil, 0, errors.New("invalid rol: " + p) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 5f1c96cb91..1cf205a23f 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -50,6 +50,13 @@ const ( scopeCacheExpiration = 3600 ) +var roleRankings = map[authpb.Role]int{ + authpb.Role_ROLE_VIEWER: 0, + authpb.Role_ROLE_UPLOADER: 1, + authpb.Role_ROLE_EDITOR: 2, + authpb.Role_ROLE_OWNER: 3, +} + func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[string]*authpb.Scope, user *userpb.User, gatewayAddr string, mgr token.Manager) error { log := appctx.GetLogger(ctx) client, err := pool.GetGatewayServiceClient(gatewayAddr) @@ -57,15 +64,15 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s return err } - hasEditorRole := false + highestRole := authpb.Role_ROLE_VIEWER for _, v := range tokenScope { - if v.Role == authpb.Role_ROLE_OWNER || v.Role == authpb.Role_ROLE_EDITOR { - hasEditorRole = true + if roleRankings[v.Role] > roleRankings[highestRole] { + highestRole = v.Role break } } - if ref, ok := extractRef(req, hasEditorRole); ok { + if ref, ok := extractRef(req, highestRole); ok { // The request is for a storage reference. This can be the case for multiple scenarios: // - If the path is not empty, the request might be coming from a share where the accessor is // trying to impersonate the owner, since the share manager doesn't know the @@ -240,7 +247,7 @@ func checkIfNestedResource(ctx context.Context, ref *provider.Reference, parent } -func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) { +func extractRefForReaderRole(req interface{}) (*provider.Reference, bool) { switch v := req.(type) { // Read requests case *registry.GetStorageProvidersRequest: @@ -256,15 +263,16 @@ func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) case *gateway.OpenInAppRequest: return v.GetRef(), true - // App provider requests + // App provider requests case *appregistry.GetAppProvidersRequest: return &provider.Reference{ResourceId: v.ResourceInfo.Id}, true } - if !hasEditorRole { - return nil, false - } + return nil, false +} + +func extractRefForUploaderRole(req interface{}) (*provider.Reference, bool) { switch v := req.(type) { // Write Requests case *provider.CreateContainerRequest: @@ -281,7 +289,27 @@ func extractRef(req interface{}, hasEditorRole bool) (*provider.Reference, bool) return v.GetRef(), true case *provider.UnsetArbitraryMetadataRequest: return v.GetRef(), true + } + return nil, false + +} + +func extractRef(req interface{}, role authpb.Role) (*provider.Reference, bool) { + switch role { + case authpb.Role_ROLE_UPLOADER: + return extractRefForUploaderRole(req) + case authpb.Role_ROLE_VIEWER: + return extractRefForReaderRole(req) + default: // Owner or editor role + ref, ok := extractRefForReaderRole(req) + if ok { + return ref, true + } + ref, ok = extractRefForUploaderRole(req) + if ok { + return ref, true + } } return nil, false } diff --git a/internal/http/services/owncloud/ocs/conversions/role.go b/internal/http/services/owncloud/ocs/conversions/role.go index f04bd7cabd..3ede7b81df 100644 --- a/internal/http/services/owncloud/ocs/conversions/role.go +++ b/internal/http/services/owncloud/ocs/conversions/role.go @@ -242,7 +242,7 @@ func NewCoownerRole() *Role { // NewUploaderRole creates an uploader role func NewUploaderRole() *Role { return &Role{ - Name: RoleViewer, + Name: RoleUploader, cS3ResourcePermissions: &provider.ResourcePermissions{ Stat: true, ListContainer: true, diff --git a/pkg/auth/manager/publicshares/publicshares.go b/pkg/auth/manager/publicshares/publicshares.go index 0dc4de28be..bbbc30a25a 100644 --- a/pkg/auth/manager/publicshares/publicshares.go +++ b/pkg/auth/manager/publicshares/publicshares.go @@ -137,10 +137,14 @@ func (m *manager) Authenticate(ctx context.Context, token, secret string) (*user share := publicShareResponse.GetShare() role := authpb.Role_ROLE_VIEWER roleStr := "viewer" - if share.Permissions.Permissions.InitiateFileUpload { + if share.Permissions.Permissions.InitiateFileUpload && !share.Permissions.Permissions.InitiateFileDownload { + role = authpb.Role_ROLE_UPLOADER + roleStr = "uploader" + } else if share.Permissions.Permissions.InitiateFileUpload { role = authpb.Role_ROLE_EDITOR roleStr = "editor" } + scope, err := scope.AddPublicShareScope(share, role, nil) if err != nil { return nil, nil, err diff --git a/pkg/auth/scope/scope.go b/pkg/auth/scope/scope.go index 4c4ddc2c67..c040854e80 100644 --- a/pkg/auth/scope/scope.go +++ b/pkg/auth/scope/scope.go @@ -56,5 +56,5 @@ func VerifyScope(ctx context.Context, scopeMap map[string]*authpb.Scope, resourc } func hasRoleEditor(scope authpb.Scope) bool { - return scope.Role == authpb.Role_ROLE_EDITOR + return scope.Role == authpb.Role_ROLE_OWNER || scope.Role == authpb.Role_ROLE_EDITOR || scope.Role == authpb.Role_ROLE_UPLOADER } diff --git a/pkg/cbox/utils/conversions.go b/pkg/cbox/utils/conversions.go index 271f8d5cb1..41b8e78a44 100644 --- a/pkg/cbox/utils/conversions.go +++ b/pkg/cbox/utils/conversions.go @@ -28,6 +28,7 @@ import ( link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" ) // DBShare stores information about user and public shares. @@ -129,46 +130,14 @@ func SharePermToInt(p *provider.ResourcePermissions) int { func IntTosharePerm(p int, itemType string) *provider.ResourcePermissions { switch p { case 1: - return &provider.ResourcePermissions{ - ListContainer: true, - ListGrants: true, - ListFileVersions: true, - ListRecycle: true, - Stat: true, - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - } + return conversions.NewViewerRole().CS3ResourcePermissions() case 15: - perm := &provider.ResourcePermissions{ - ListContainer: true, - ListGrants: true, - ListFileVersions: true, - ListRecycle: true, - Stat: true, - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - - InitiateFileUpload: true, - RestoreFileVersion: true, - RestoreRecycleItem: true, - } if itemType == "folder" { - perm.CreateContainer = true - perm.Delete = true - perm.Move = true - perm.PurgeRecycle = true + return conversions.NewEditorRole().CS3ResourcePermissions() } - return perm + return conversions.NewFileEditorRole().CS3ResourcePermissions() case 4: - return &provider.ResourcePermissions{ - Stat: true, - ListContainer: true, - GetPath: true, - CreateContainer: true, - InitiateFileUpload: true, - } + return conversions.NewUploaderRole().CS3ResourcePermissions() default: // TODO we may have other options, for now this is a denial return &provider.ResourcePermissions{} diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index c8c44b7c56..4e6bbab6e0 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -38,6 +38,7 @@ 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/internal/http/services/owncloud/ocs/conversions" "github.com/cs3org/reva/pkg/appctx" ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/eosclient" @@ -1677,59 +1678,17 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI if u.Opaque != nil { if publicShare, ok := u.Opaque.Map["public-share-role"]; ok { if string(publicShare.Value) == "editor" { - return &provider.ResourcePermissions{ - CreateContainer: true, - Delete: true, - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - InitiateFileUpload: true, - ListContainer: true, - ListFileVersions: true, - ListGrants: true, - ListRecycle: true, - Move: true, - PurgeRecycle: true, - RestoreFileVersion: true, - RestoreRecycleItem: true, - Stat: true, - } - } - return &provider.ResourcePermissions{ - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - ListContainer: true, - ListFileVersions: true, - ListRecycle: true, - ListGrants: true, - Stat: true, + return conversions.NewEditorRole().CS3ResourcePermissions() + } else if string(publicShare.Value) == "uploader" { + return conversions.NewUploaderRole().CS3ResourcePermissions() } + // Default to viewer role + return conversions.NewViewerRole().CS3ResourcePermissions() } } - return &provider.ResourcePermissions{ - // owner has all permissions - AddGrant: true, - CreateContainer: true, - Delete: true, - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - InitiateFileUpload: true, - ListContainer: true, - ListFileVersions: true, - ListGrants: true, - ListRecycle: true, - Move: true, - PurgeRecycle: true, - RemoveGrant: true, - RestoreFileVersion: true, - RestoreRecycleItem: true, - Stat: true, - UpdateGrant: true, - DenyGrant: true, - } + // owner has all permissions + return conversions.NewManagerRole().CS3ResourcePermissions() } auth, err := fs.getUserAuth(ctx, u, eosFileInfo.File) From 7a7a3b296b3d0a71151830ce71e4eed089152b25 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Fri, 28 Jan 2022 10:08:50 +0100 Subject: [PATCH 2/7] Enable deny grant only for project spaces --- pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go | 5 +++++ pkg/cbox/storage/eoswrapper/eoswrapper.go | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go index 7160967ff8..39a021d92b 100644 --- a/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go +++ b/pkg/cbox/storage/eoshomewrapper/eoshomewrapper.go @@ -26,6 +26,7 @@ import ( "github.com/Masterminds/sprig" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/storage" "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/cs3org/reva/pkg/storage/utils/eosfs" @@ -112,6 +113,10 @@ func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKey return res, nil } +func (w *wrapper) DenyGrant(ctx context.Context, ref *provider.Reference, g *provider.Grantee) error { + return errtypes.NotSupported("eos: deny grant is only enabled for project spaces") +} + func (w *wrapper) getMountID(ctx context.Context, r *provider.ResourceInfo) string { u := ctxpkg.ContextMustGetUser(ctx) b := bytes.Buffer{} diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index 5cbfc0feed..4ec0413cf0 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -162,6 +162,18 @@ func (w *wrapper) RestoreRevision(ctx context.Context, ref *provider.Reference, return w.FS.RestoreRevision(ctx, ref, revisionKey) } +func (w *wrapper) DenyGrant(ctx context.Context, ref *provider.Reference, g *provider.Grantee) error { + // This is only allowed for project space admins + if strings.HasPrefix(w.conf.Namespace, eosProjectsNamespace) { + if err := w.userIsProjectAdmin(ctx, ref); err != nil { + return err + } + return w.FS.DenyGrant(ctx, ref, g) + } + + return errtypes.NotSupported("eos: deny grant is only enabled for project spaces") +} + func (w *wrapper) getMountID(ctx context.Context, r *provider.ResourceInfo) string { if r == nil { return "" @@ -194,6 +206,7 @@ func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider. r.PermissionSet.UpdateGrant = true r.PermissionSet.ListGrants = true r.PermissionSet.GetQuota = true + r.PermissionSet.DenyGrant = true return nil } } From 7979cadac2019660eac1fca009243b739bdb44da Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Fri, 25 Feb 2022 13:49:29 +0100 Subject: [PATCH 3/7] Refactor ctx user agent interceptor and add desktop client ns --- .../grpc/services/gateway/storageprovider.go | 30 ++++---- internal/http/interceptors/auth/auth.go | 4 +- .../http/services/owncloud/ocdav/ocdav.go | 6 ++ pkg/ctx/agentctx.go | 53 ++++++++++++- .../agentctx_test.go} | 73 ++++-------------- pkg/useragent/useragent.go | 77 ------------------- 6 files changed, 92 insertions(+), 151 deletions(-) rename pkg/{useragent/useragent_test.go => ctx/agentctx_test.go} (50%) delete mode 100644 pkg/useragent/useragent.go diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 54c0230f9b..912fc72d1b 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -37,8 +37,6 @@ import ( types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" ctxpkg "github.com/cs3org/reva/pkg/ctx" rtrace "github.com/cs3org/reva/pkg/trace" - "github.com/cs3org/reva/pkg/useragent" - ua "github.com/mileusna/useragent" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/errtypes" @@ -1778,30 +1776,36 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp return lcr, nil } -func (s *svc) isPathAllowed(ua *ua.UserAgent, path string) bool { - uaLst, ok := s.c.AllowedUserAgents[path] - if !ok { - // if no user agent is defined for a path, all user agents are allowed - return true - } - return useragent.IsUserAgentAllowed(ua, uaLst) -} - func (s *svc) filterProvidersByUserAgent(ctx context.Context, providers []*registry.ProviderInfo) []*registry.ProviderInfo { - ua, ok := ctxpkg.ContextGetUserAgent(ctx) + cat, ok := ctxpkg.ContextGetUserAgentCategory(ctx) if !ok { return providers } filters := []*registry.ProviderInfo{} for _, p := range providers { - if s.isPathAllowed(ua, p.ProviderPath) { + if s.isPathAllowed(cat, p.ProviderPath) { filters = append(filters, p) } } return filters } +func (s *svc) isPathAllowed(cat string, path string) bool { + allowedUserAgents, ok := s.c.AllowedUserAgents[path] + if !ok { + // if no user agent is defined for a path, all user agents are allowed + return true + } + + for _, userAgent := range allowedUserAgents { + if userAgent == cat { + return true + } + } + return false +} + func (s *svc) listContainer(ctx context.Context, req *provider.ListContainerRequest) (*provider.ListContainerResponse, error) { providers, err := s.findProviders(ctx, req.Ref) if err != nil { diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index c73b8afacb..cb4c55c6a0 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -173,7 +173,7 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err isUnprotectedEndpoint = true } - ctx, err := authenticateUser(w, r, conf, unprotected, tokenStrategy, tokenManager, tokenWriter, credChain, isUnprotectedEndpoint) + ctx, err := authenticateUser(w, r, conf, tokenStrategy, tokenManager, tokenWriter, credChain, isUnprotectedEndpoint) if err != nil { if !isUnprotectedEndpoint { return @@ -187,7 +187,7 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err return chain, nil } -func authenticateUser(w http.ResponseWriter, r *http.Request, conf *config, unprotected []string, tokenStrategy auth.TokenStrategy, tokenManager token.Manager, tokenWriter auth.TokenWriter, credChain map[string]auth.CredentialStrategy, isUnprotectedEndpoint bool) (context.Context, error) { +func authenticateUser(w http.ResponseWriter, r *http.Request, conf *config, tokenStrategy auth.TokenStrategy, tokenManager token.Manager, tokenWriter auth.TokenWriter, credChain map[string]auth.CredentialStrategy, isUnprotectedEndpoint bool) (context.Context, error) { ctx := r.Context() log := appctx.GetLogger(ctx) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index e2aa3c0ca5..6d3cadc0f1 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -203,6 +203,12 @@ func (s *svc) Handler() http.Handler { head, r.URL.Path = router.ShiftPath(r.URL.Path) log.Debug().Str("head", head).Str("tail", r.URL.Path).Msg("http routing") switch head { + case "cernbox": + var origin string + origin, r.URL.Path = router.ShiftPath(r.URL.Path) // desktop or mobile + _, r.URL.Path = router.ShiftPath(r.URL.Path) // remote.php + head, r.URL.Path = router.ShiftPath(r.URL.Path) + base = path.Join(base, "cernbox", origin, "remote.php") case "status.php": s.doStatus(w, r) return diff --git a/pkg/ctx/agentctx.go b/pkg/ctx/agentctx.go index 74eda0b563..1d96f355af 100644 --- a/pkg/ctx/agentctx.go +++ b/pkg/ctx/agentctx.go @@ -20,13 +20,21 @@ package ctx import ( "context" + "strings" ua "github.com/mileusna/useragent" "google.golang.org/grpc/metadata" ) // UserAgentHeader is the header used for the user agent -const UserAgentHeader = "x-user-agent" +const ( + UserAgentHeader = "x-user-agent" + + WebUserAgent = "web" + GrpcUserAgent = "grpc" + MobileUserAgent = "mobile" + DesktopUserAgent = "desktop" +) // ContextGetUserAgent returns the user agent if set in the given context. // see https://github.com/grpc/grpc-go/issues/1100 @@ -56,3 +64,46 @@ func ContextGetUserAgentString(ctx context.Context) (string, bool) { } return userAgentLst[0], true } + +// ContextGetUserAgentCategory returns the category of the user agent +// (i.e. if it is a web, mobile, desktop or grpc user agent) +func ContextGetUserAgentCategory(ctx context.Context) (string, bool) { + agent, ok := ContextGetUserAgent(ctx) + if !ok { + return "", false + } + switch { + case isWeb(agent): + return WebUserAgent, true + case isMobile(agent): + return MobileUserAgent, true + case isDesktop(agent): + return DesktopUserAgent, true + case isGRPC(agent): + return GrpcUserAgent, true + default: + return "", false + } +} + +func isWeb(ua *ua.UserAgent) bool { + return ua.IsChrome() || ua.IsEdge() || ua.IsFirefox() || ua.IsSafari() || + ua.IsInternetExplorer() || ua.IsOpera() || ua.IsOperaMini() +} + +// isMobile returns true if the useragent is generated by the mobile +func isMobile(ua *ua.UserAgent) bool { + // workaround as the library does not recognise iOS string inside the user agent + isIOS := ua.IsIOS() || strings.Contains(ua.String, "iOS") + return !isWeb(ua) && (ua.IsAndroid() || isIOS) +} + +// isDesktop returns true if the useragent is generated by a desktop application +func isDesktop(ua *ua.UserAgent) bool { + return ua.Desktop && !isWeb(ua) +} + +// isGRPC returns true if the useragent is generated by a grpc client +func isGRPC(ua *ua.UserAgent) bool { + return strings.HasPrefix(ua.Name, "grpc") +} diff --git a/pkg/useragent/useragent_test.go b/pkg/ctx/agentctx_test.go similarity index 50% rename from pkg/useragent/useragent_test.go rename to pkg/ctx/agentctx_test.go index c37ae97508..8cfd6b0e6a 100644 --- a/pkg/useragent/useragent_test.go +++ b/pkg/ctx/agentctx_test.go @@ -16,12 +16,13 @@ // granted to it by virtue of its status as an Intergovernmental Organization // or submit itself to any jurisdiction. -package useragent +package ctx import ( + "context" "testing" - ua "github.com/mileusna/useragent" + "google.golang.org/grpc/metadata" ) func TestUserAgentIsAllowed(t *testing.T) { @@ -29,92 +30,48 @@ func TestUserAgentIsAllowed(t *testing.T) { tests := []struct { description string userAgent string - userAgents []string - expected bool + expected string }{ { description: "grpc-go", userAgent: "grpc-go", - userAgents: []string{"grpc"}, - expected: true, - }, - { - description: "grpc-go", - userAgent: "grpc-go", - userAgents: []string{"desktop", "mobile", "web"}, - expected: false, + expected: "grpc", }, { description: "web-firefox", userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", - userAgents: []string{"web"}, - expected: true, - }, - { - description: "web-firefox", - userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15", - userAgents: []string{"desktop", "mobile", "grpc"}, - expected: false, - }, - { - description: "desktop-mirall", - userAgent: "Mozilla/5.0 (Linux) mirall/2.7.1 (build 2596) (cernboxcmd, centos-3.10.0-1160.36.2.el7.x86_64 ClientArchitecture: x86_64 OsArchitecture: x86_64)", - userAgents: []string{"desktop"}, - expected: true, + expected: "web", }, { description: "desktop-mirall", userAgent: "Mozilla/5.0 (Linux) mirall/2.7.1 (build 2596) (cernboxcmd, centos-3.10.0-1160.36.2.el7.x86_64 ClientArchitecture: x86_64 OsArchitecture: x86_64)", - userAgents: []string{"web", "mobile", "grpc"}, - expected: false, - }, - { - description: "mobile-android", - userAgent: "Mozilla/5.0 (Android) ownCloud-android/2.13.1 cernbox/Android", - userAgents: []string{"mobile"}, - expected: true, - }, - { - description: "mobile-ios", - userAgent: "Mozilla/5.0 (iOS) ownCloud-iOS/3.8.0 cernbox/iOS", - userAgents: []string{"mobile"}, - expected: true, + expected: "desktop", }, { description: "mobile-android", userAgent: "Mozilla/5.0 (Android) ownCloud-android/2.13.1 cernbox/Android", - userAgents: []string{"web", "desktop", "grpc"}, - expected: false, + expected: "mobile", }, { description: "mobile-ios", userAgent: "Mozilla/5.0 (iOS) ownCloud-iOS/3.8.0 cernbox/iOS", - userAgents: []string{"web", "desktop", "grpc"}, - expected: false, + expected: "mobile", }, { description: "mobile-web", userAgent: "Mozilla/5.0 (Android 11; Mobile; rv:86.0) Gecko/86.0 Firefox/86.0", - userAgents: []string{"web"}, - expected: true, - }, - { - description: "mobile-web", - userAgent: "Mozilla/5.0 (Android 11; Mobile; rv:86.0) Gecko/86.0 Firefox/86.0", - userAgents: []string{"desktop", "grpc", "mobile"}, - expected: false, + expected: "web", }, } + ctx := context.Background() for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { + ctx = metadata.NewIncomingContext(ctx, metadata.New(map[string]string{UserAgentHeader: tt.userAgent})) + cat, _ := ContextGetUserAgentCategory(ctx) - ua := ua.Parse(tt.userAgent) - - res := IsUserAgentAllowed(&ua, tt.userAgents) - - if res != tt.expected { - t.Fatalf("result does not match with expected. got=%+v expected=%+v", res, tt.expected) + if cat != tt.expected { + t.Fatalf("result does not match with expected. got=%+v expected=%+v", cat, tt.expected) } }) diff --git a/pkg/useragent/useragent.go b/pkg/useragent/useragent.go deleted file mode 100644 index 53d766ed26..0000000000 --- a/pkg/useragent/useragent.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2018-2021 CERN -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// In applying this license, CERN does not waive the privileges and immunities -// granted to it by virtue of its status as an Intergovernmental Organization -// or submit itself to any jurisdiction. - -package useragent - -import ( - "strings" - - ua "github.com/mileusna/useragent" -) - -// isWeb returns true if the useragent is generated by the web -func isWeb(ua *ua.UserAgent) bool { - return ua.IsChrome() || ua.IsEdge() || ua.IsFirefox() || ua.IsSafari() || - ua.IsInternetExplorer() || ua.IsOpera() || ua.IsOperaMini() -} - -// isMobile returns true if the useragent is generated by the mobile -func isMobile(ua *ua.UserAgent) bool { - // workaround as the library does not recognise iOS string inside the user agent - isIOS := ua.IsIOS() || strings.Contains(ua.String, "iOS") - return !isWeb(ua) && (ua.IsAndroid() || isIOS) -} - -// isDesktop returns true if the useragent is generated by a desktop application -func isDesktop(ua *ua.UserAgent) bool { - return ua.Desktop && !isWeb(ua) -} - -// isGRPC returns true if the useragent is generated by a grpc client -func isGRPC(ua *ua.UserAgent) bool { - return strings.HasPrefix(ua.Name, "grpc") -} - -// getCategory returns the category of the user agent -// (i.e. if it is a web, mobile, desktop or grpc user agent) -func getCategory(ua *ua.UserAgent) string { - switch { - case isWeb(ua): - return "web" - case isMobile(ua): - return "mobile" - case isDesktop(ua): - return "desktop" - case isGRPC(ua): - return "grpc" - default: - return "" - } -} - -// IsUserAgentAllowed return true if the user agent corresponds -// to one in the allowed user agents list -func IsUserAgentAllowed(ua *ua.UserAgent, allowedUserAgents []string) bool { - cat := getCategory(ua) - for _, userAgent := range allowedUserAgents { - if userAgent == cat { - return true - } - } - return false -} From 7a1083df004dbc9ddc341e062c85ec3e7c5a9f03 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 2 Mar 2022 15:55:14 +0100 Subject: [PATCH 4/7] Handle share references conflicts --- .../grpc/services/gateway/ocmshareprovider.go | 73 ++++++++++- .../grpc/services/gateway/storageprovider.go | 5 + .../services/gateway/usershareprovider.go | 122 +++++++++++------- 3 files changed, 151 insertions(+), 49 deletions(-) diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go index d0675b7c6e..975b17d4fa 100644 --- a/internal/grpc/services/gateway/ocmshareprovider.go +++ b/internal/grpc/services/gateway/ocmshareprovider.go @@ -391,7 +391,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive Status: createRefStatus, }, err case ocm.ShareState_SHARE_STATE_REJECTED: - s.removeReference(ctx, req.GetShare().GetShare().ResourceId) // error is logged inside removeReference + s.removeOCMReference(ctx, req.GetShare().GetShare().GetResourceId()) // error is logged inside removeReference // FIXME we are ignoring an error from removeReference here return res, nil } @@ -425,6 +425,77 @@ func (s *svc) GetReceivedOCMShare(ctx context.Context, req *ocm.GetReceivedOCMSh return res, nil } +func (s *svc) removeOCMReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { + log := appctx.GetLogger(ctx) + + idReference := &provider.Reference{ResourceId: resourceID} + storageProvider, err := s.find(ctx, idReference) + if err != nil { + if _, ok := err.(errtypes.IsNotFound); ok { + return status.NewNotFound(ctx, "storage provider not found") + } + return status.NewInternal(ctx, err, "error finding storage provider") + } + + statRes, err := storageProvider.Stat(ctx, &provider.StatRequest{Ref: idReference}) + if err != nil { + return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) + } + + // FIXME how can we delete a reference if the original resource was deleted? + if statRes.Status.Code != rpc.Code_CODE_OK { + err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway") + return status.NewInternal(ctx, err, "could not delete share reference") + } + + homeRes, err := s.GetHome(ctx, &provider.GetHomeRequest{}) + if err != nil { + err := errors.Wrap(err, "gateway: error calling GetHome") + return status.NewInternal(ctx, err, "could not delete share reference") + } + + sharePath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) + log.Debug().Str("share_path", sharePath).Msg("remove reference of share") + + homeProvider, err := s.find(ctx, &provider.Reference{Path: sharePath}) + if err != nil { + if _, ok := err.(errtypes.IsNotFound); ok { + return status.NewNotFound(ctx, "storage provider not found") + } + return status.NewInternal(ctx, err, "error finding storage provider") + } + + deleteReq := &provider.DeleteRequest{ + Opaque: &types.Opaque{ + Map: map[string]*types.OpaqueEntry{ + // This signals the storageprovider that we want to delete the share reference and not the underlying file. + "deleting_shared_resource": {}, + }, + }, + Ref: &provider.Reference{Path: sharePath}, + } + + deleteResp, err := homeProvider.Delete(ctx, deleteReq) + if err != nil { + return status.NewInternal(ctx, err, "could not delete share reference") + } + + switch deleteResp.Status.Code { + case rpc.Code_CODE_OK: + // we can continue deleting the reference + case rpc.Code_CODE_NOT_FOUND: + // This is fine, we wanted to delete it anyway + return status.NewOK(ctx) + default: + err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") + return status.NewInternal(ctx, err, "could not delete share reference") + } + + log.Debug().Str("share_path", sharePath).Msg("share reference successfully removed") + + return status.NewOK(ctx) +} + func (s *svc) createOCMReference(ctx context.Context, share *ocm.Share) (*rpc.Status, error) { log := appctx.GetLogger(ctx) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 912fc72d1b..413ee827d9 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -35,6 +35,7 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" ctxpkg "github.com/cs3org/reva/pkg/ctx" rtrace "github.com/cs3org/reva/pkg/trace" @@ -1555,6 +1556,8 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St orgPath := res.Info.Path res.Info = ri res.Info.Path = orgPath + // It should be possible to delete and move share references, so expose all possible permissions + res.Info.PermissionSet = conversions.NewManagerRole().CS3ResourcePermissions() return res, nil } @@ -1768,6 +1771,8 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp } } + // It should be possible to delete and move share references, so expose all possible permissions + info.PermissionSet = conversions.NewManagerRole().CS3ResourcePermissions() info.Path = lcr.Infos[i].Path checkedInfos = append(checkedInfos, info) } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index a161e22233..b173748fb8 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -34,6 +34,7 @@ import ( "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/storage/utils/grants" + "github.com/cs3org/reva/pkg/utils" "github.com/pkg/errors" ) @@ -130,8 +131,6 @@ func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareReq return nil, errors.Wrap(err, "gateway: error calling RemoveShare") } - s.removeReference(ctx, share.ResourceId) - // if we don't need to commit we return earlier if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { return res, nil @@ -346,12 +345,12 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update case "state": switch req.GetShare().GetState() { case collaboration.ShareState_SHARE_STATE_ACCEPTED: - rpcStatus := s.createReference(ctx, res.GetShare().GetShare().GetResourceId()) + rpcStatus := s.createReference(ctx, res.GetShare().GetShare()) if rpcStatus.Code != rpc.Code_CODE_OK { return &collaboration.UpdateReceivedShareResponse{Status: rpcStatus}, nil } case collaboration.ShareState_SHARE_STATE_REJECTED: - rpcStatus := s.removeReference(ctx, res.GetShare().GetShare().ResourceId) + rpcStatus := s.removeReference(ctx, res.GetShare().GetShare()) if rpcStatus.Code != rpc.Code_CODE_OK && rpcStatus.Code != rpc.Code_CODE_NOT_FOUND { return &collaboration.UpdateReceivedShareResponse{Status: rpcStatus}, nil } @@ -369,10 +368,10 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update return res, nil } -func (s *svc) removeReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { +func (s *svc) removeReference(ctx context.Context, share *collaboration.Share) *rpc.Status { log := appctx.GetLogger(ctx) - idReference := &provider.Reference{ResourceId: resourceID} + idReference := &provider.Reference{ResourceId: share.ResourceId} storageProvider, err := s.find(ctx, idReference) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { @@ -383,7 +382,7 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource statRes, err := storageProvider.Stat(ctx, &provider.StatRequest{Ref: idReference}) if err != nil { - return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) + return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+share.ResourceId.String()) } // FIXME how can we delete a reference if the original resource was deleted? @@ -398,51 +397,68 @@ func (s *svc) removeReference(ctx context.Context, resourceID *provider.Resource return status.NewInternal(ctx, err, "could not delete share reference") } - sharePath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) - log.Debug().Str("share_path", sharePath).Msg("remove reference of share") + // We list the shares folder and delete references corresponding to this share + // TODO: We need to maintain a DB of these + shareFolderPath := path.Join(homeRes.Path, s.c.ShareFolder) - homeProvider, err := s.find(ctx, &provider.Reference{Path: sharePath}) + homeProvider, err := s.find(ctx, &provider.Reference{Path: shareFolderPath}) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { return status.NewNotFound(ctx, "storage provider not found") } return status.NewInternal(ctx, err, "error finding storage provider") } - - deleteReq := &provider.DeleteRequest{ - Opaque: &typesv1beta1.Opaque{ - Map: map[string]*typesv1beta1.OpaqueEntry{ - // This signals the storageprovider that we want to delete the share reference and not the underlying file. - "deleting_shared_resource": {}, - }, - }, - Ref: &provider.Reference{Path: sharePath}, - } - - deleteResp, err := homeProvider.Delete(ctx, deleteReq) + shareRefs, err := homeProvider.ListContainer(ctx, &provider.ListContainerRequest{ + Ref: &provider.Reference{Path: shareFolderPath}, + }) if err != nil { - return status.NewInternal(ctx, err, "could not delete share reference") + return status.NewInternal(ctx, err, "gateway: error listing shares folder") } - - switch deleteResp.Status.Code { - case rpc.Code_CODE_OK: - // we can continue deleting the reference - case rpc.Code_CODE_NOT_FOUND: - // This is fine, we wanted to delete it anyway - return status.NewOK(ctx) - default: - err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") - return status.NewInternal(ctx, err, "could not delete share reference") + if shareRefs.Status.Code != rpc.Code_CODE_OK { + err := status.NewErrorFromCode(shareRefs.Status.GetCode(), "gateway") + return status.NewInternal(ctx, err, "could not list shares folder") } - log.Debug().Str("share_path", sharePath).Msg("share reference successfully removed") + target := fmt.Sprintf("cs3:%s/%s", share.ResourceId.GetStorageId(), share.ResourceId.GetOpaqueId()) + for _, s := range shareRefs.Infos { + if s.Target == target { + log.Debug().Str("share_path", s.Path).Msg("remove reference of share") + + deleteReq := &provider.DeleteRequest{ + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + // This signals the storageprovider that we want to delete the share reference and not the underlying file. + "deleting_shared_resource": {}, + }, + }, + Ref: &provider.Reference{Path: s.Path}, + } + + deleteResp, err := homeProvider.Delete(ctx, deleteReq) + if err != nil { + return status.NewInternal(ctx, err, "could not delete share reference") + } + + switch deleteResp.Status.Code { + case rpc.Code_CODE_OK: + // we can continue deleting the reference + case rpc.Code_CODE_NOT_FOUND: + // This is fine, we wanted to delete it anyway + return status.NewOK(ctx) + default: + err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") + return status.NewInternal(ctx, err, "could not delete share reference") + } + log.Debug().Str("share_path", s.Path).Msg("share reference successfully removed") + } + } return status.NewOK(ctx) } -func (s *svc) createReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { +func (s *svc) createReference(ctx context.Context, share *collaboration.Share) *rpc.Status { ref := &provider.Reference{ - ResourceId: resourceID, + ResourceId: share.ResourceId, } log := appctx.GetLogger(ctx) @@ -461,12 +477,12 @@ func (s *svc) createReference(ctx context.Context, resourceID *provider.Resource statRes, err := c.Stat(ctx, statReq) if err != nil { - return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) + return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+share.ResourceId.String()) } if statRes.Status.Code != rpc.Code_CODE_OK { err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway") - log.Err(err).Msg("gateway: Stat failed on the share resource id: " + resourceID.String()) + log.Err(err).Msg("gateway: Stat failed on the share resource id: " + share.ResourceId.String()) return status.NewInternal(ctx, err, "error updating received share") } @@ -486,16 +502,8 @@ func (s *svc) createReference(ctx context.Context, resourceID *provider.Resource // It is the responsibility of the gateway to resolve these references and merge the response back // from the main request. // TODO(labkode): the name of the share should be the filename it points to by default. - refPath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) - log.Info().Msg("mount path will be:" + refPath) - - createRefReq := &provider.CreateReferenceRequest{ - Ref: &provider.Reference{Path: refPath}, - // cs3 is the Scheme and %s/%s is the Opaque parts of a net.URL. - TargetUri: fmt.Sprintf("cs3:%s/%s", resourceID.GetStorageId(), resourceID.GetOpaqueId()), - } - - c, err = s.findByPath(ctx, refPath) + refPath := &provider.Reference{Path: path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path))} + c, err = s.findByPath(ctx, refPath.Path) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { return status.NewNotFound(ctx, "storage provider not found") @@ -503,6 +511,24 @@ func (s *svc) createReference(ctx context.Context, resourceID *provider.Resource return status.NewInternal(ctx, err, "error finding storage provider") } + refPathStat, err := c.Stat(ctx, &provider.StatRequest{ + Ref: refPath, + }) + if err == nil && refPathStat.Status.Code == rpc.Code_CODE_OK { + // This reference already exists, add extra metadata to avoid conflicts + name := fmt.Sprintf("%s_%s_%s", path.Base(statRes.Info.Path), share.Owner.OpaqueId, utils.TSToTime(share.Ctime).Format("2006-01-02_15-04-05")) + refPath = &provider.Reference{Path: path.Join(homeRes.Path, s.c.ShareFolder, name)} + } + + log.Info().Msgf("refPathStat %+v %+v", refPathStat.Status, err) + log.Info().Msg("mount path will be:" + refPath.Path) + + createRefReq := &provider.CreateReferenceRequest{ + Ref: refPath, + // cs3 is the Scheme and %s/%s is the Opaque parts of a net.URL. + TargetUri: fmt.Sprintf("cs3:%s/%s", share.ResourceId.GetStorageId(), share.ResourceId.GetOpaqueId()), + } + createRefRes, err := c.CreateReference(ctx, createRefReq) if err != nil { log.Err(err).Msg("gateway: error calling GetHome") From f21dfb532b3d1d9b78b5b77c5aca39690b701644 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 19 Apr 2022 16:06:17 +0200 Subject: [PATCH 5/7] Fix scope checks --- internal/grpc/interceptors/auth/scope.go | 63 ++++++++++++++---------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 1cf205a23f..7cf59f1d8e 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -50,13 +50,6 @@ const ( scopeCacheExpiration = 3600 ) -var roleRankings = map[authpb.Role]int{ - authpb.Role_ROLE_VIEWER: 0, - authpb.Role_ROLE_UPLOADER: 1, - authpb.Role_ROLE_EDITOR: 2, - authpb.Role_ROLE_OWNER: 3, -} - func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[string]*authpb.Scope, user *userpb.User, gatewayAddr string, mgr token.Manager) error { log := appctx.GetLogger(ctx) client, err := pool.GetGatewayServiceClient(gatewayAddr) @@ -64,15 +57,7 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s return err } - highestRole := authpb.Role_ROLE_VIEWER - for _, v := range tokenScope { - if roleRankings[v.Role] > roleRankings[highestRole] { - highestRole = v.Role - break - } - } - - if ref, ok := extractRef(req, highestRole); ok { + if ref, ok := extractRef(req, tokenScope); ok { // The request is for a storage reference. This can be the case for multiple scenarios: // - If the path is not empty, the request might be coming from a share where the accessor is // trying to impersonate the owner, since the share manager doesn't know the @@ -279,12 +264,21 @@ func extractRefForUploaderRole(req interface{}) (*provider.Reference, bool) { return v.GetRef(), true case *provider.TouchFileRequest: return v.GetRef(), true + case *provider.InitiateFileUploadRequest: + return v.GetRef(), true + } + + return nil, false + +} + +func extractRefForEditorRole(req interface{}) (*provider.Reference, bool) { + switch v := req.(type) { + // Remaining edit Requests case *provider.DeleteRequest: return v.GetRef(), true case *provider.MoveRequest: return v.GetSource(), true - case *provider.InitiateFileUploadRequest: - return v.GetRef(), true case *provider.SetArbitraryMetadataRequest: return v.GetRef(), true case *provider.UnsetArbitraryMetadataRequest: @@ -295,22 +289,39 @@ func extractRefForUploaderRole(req interface{}) (*provider.Reference, bool) { } -func extractRef(req interface{}, role authpb.Role) (*provider.Reference, bool) { - switch role { - case authpb.Role_ROLE_UPLOADER: - return extractRefForUploaderRole(req) - case authpb.Role_ROLE_VIEWER: - return extractRefForReaderRole(req) - default: // Owner or editor role +func extractRef(req interface{}, tokenScope map[string]*authpb.Scope) (*provider.Reference, bool) { + var readPerm, uploadPerm, editPerm bool + for _, v := range tokenScope { + if v.Role == authpb.Role_ROLE_OWNER || v.Role == authpb.Role_ROLE_EDITOR || v.Role == authpb.Role_ROLE_VIEWER { + readPerm = true + } + if v.Role == authpb.Role_ROLE_OWNER || v.Role == authpb.Role_ROLE_EDITOR || v.Role == authpb.Role_ROLE_UPLOADER { + uploadPerm = true + } + if v.Role == authpb.Role_ROLE_OWNER || v.Role == authpb.Role_ROLE_EDITOR { + editPerm = true + } + } + + if readPerm { ref, ok := extractRefForReaderRole(req) if ok { return ref, true } - ref, ok = extractRefForUploaderRole(req) + } + if uploadPerm { + ref, ok := extractRefForUploaderRole(req) if ok { return ref, true } } + if editPerm { + ref, ok := extractRefForEditorRole(req) + if ok { + return ref, true + } + } + return nil, false } From 1672733874d812f3daf6afef32a2aa1919c230b7 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 19 Apr 2022 16:41:00 +0200 Subject: [PATCH 6/7] Revert "Handle share references conflicts" This reverts commit 7a1083df004dbc9ddc341e062c85ec3e7c5a9f03. --- .../grpc/services/gateway/ocmshareprovider.go | 73 +---------- .../grpc/services/gateway/storageprovider.go | 5 - .../services/gateway/usershareprovider.go | 122 +++++++----------- .../http/services/owncloud/ocdav/ocdav.go | 6 - 4 files changed, 49 insertions(+), 157 deletions(-) diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go index 975b17d4fa..d0675b7c6e 100644 --- a/internal/grpc/services/gateway/ocmshareprovider.go +++ b/internal/grpc/services/gateway/ocmshareprovider.go @@ -391,7 +391,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive Status: createRefStatus, }, err case ocm.ShareState_SHARE_STATE_REJECTED: - s.removeOCMReference(ctx, req.GetShare().GetShare().GetResourceId()) // error is logged inside removeReference + s.removeReference(ctx, req.GetShare().GetShare().ResourceId) // error is logged inside removeReference // FIXME we are ignoring an error from removeReference here return res, nil } @@ -425,77 +425,6 @@ func (s *svc) GetReceivedOCMShare(ctx context.Context, req *ocm.GetReceivedOCMSh return res, nil } -func (s *svc) removeOCMReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { - log := appctx.GetLogger(ctx) - - idReference := &provider.Reference{ResourceId: resourceID} - storageProvider, err := s.find(ctx, idReference) - if err != nil { - if _, ok := err.(errtypes.IsNotFound); ok { - return status.NewNotFound(ctx, "storage provider not found") - } - return status.NewInternal(ctx, err, "error finding storage provider") - } - - statRes, err := storageProvider.Stat(ctx, &provider.StatRequest{Ref: idReference}) - if err != nil { - return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) - } - - // FIXME how can we delete a reference if the original resource was deleted? - if statRes.Status.Code != rpc.Code_CODE_OK { - err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway") - return status.NewInternal(ctx, err, "could not delete share reference") - } - - homeRes, err := s.GetHome(ctx, &provider.GetHomeRequest{}) - if err != nil { - err := errors.Wrap(err, "gateway: error calling GetHome") - return status.NewInternal(ctx, err, "could not delete share reference") - } - - sharePath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) - log.Debug().Str("share_path", sharePath).Msg("remove reference of share") - - homeProvider, err := s.find(ctx, &provider.Reference{Path: sharePath}) - if err != nil { - if _, ok := err.(errtypes.IsNotFound); ok { - return status.NewNotFound(ctx, "storage provider not found") - } - return status.NewInternal(ctx, err, "error finding storage provider") - } - - deleteReq := &provider.DeleteRequest{ - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - // This signals the storageprovider that we want to delete the share reference and not the underlying file. - "deleting_shared_resource": {}, - }, - }, - Ref: &provider.Reference{Path: sharePath}, - } - - deleteResp, err := homeProvider.Delete(ctx, deleteReq) - if err != nil { - return status.NewInternal(ctx, err, "could not delete share reference") - } - - switch deleteResp.Status.Code { - case rpc.Code_CODE_OK: - // we can continue deleting the reference - case rpc.Code_CODE_NOT_FOUND: - // This is fine, we wanted to delete it anyway - return status.NewOK(ctx) - default: - err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") - return status.NewInternal(ctx, err, "could not delete share reference") - } - - log.Debug().Str("share_path", sharePath).Msg("share reference successfully removed") - - return status.NewOK(ctx) -} - func (s *svc) createOCMReference(ctx context.Context, share *ocm.Share) (*rpc.Status, error) { log := appctx.GetLogger(ctx) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 413ee827d9..912fc72d1b 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -35,7 +35,6 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" ctxpkg "github.com/cs3org/reva/pkg/ctx" rtrace "github.com/cs3org/reva/pkg/trace" @@ -1556,8 +1555,6 @@ func (s *svc) Stat(ctx context.Context, req *provider.StatRequest) (*provider.St orgPath := res.Info.Path res.Info = ri res.Info.Path = orgPath - // It should be possible to delete and move share references, so expose all possible permissions - res.Info.PermissionSet = conversions.NewManagerRole().CS3ResourcePermissions() return res, nil } @@ -1771,8 +1768,6 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp } } - // It should be possible to delete and move share references, so expose all possible permissions - info.PermissionSet = conversions.NewManagerRole().CS3ResourcePermissions() info.Path = lcr.Infos[i].Path checkedInfos = append(checkedInfos, info) } diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index b173748fb8..a161e22233 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -34,7 +34,6 @@ import ( "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/storage/utils/grants" - "github.com/cs3org/reva/pkg/utils" "github.com/pkg/errors" ) @@ -131,6 +130,8 @@ func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareReq return nil, errors.Wrap(err, "gateway: error calling RemoveShare") } + s.removeReference(ctx, share.ResourceId) + // if we don't need to commit we return earlier if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef { return res, nil @@ -345,12 +346,12 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update case "state": switch req.GetShare().GetState() { case collaboration.ShareState_SHARE_STATE_ACCEPTED: - rpcStatus := s.createReference(ctx, res.GetShare().GetShare()) + rpcStatus := s.createReference(ctx, res.GetShare().GetShare().GetResourceId()) if rpcStatus.Code != rpc.Code_CODE_OK { return &collaboration.UpdateReceivedShareResponse{Status: rpcStatus}, nil } case collaboration.ShareState_SHARE_STATE_REJECTED: - rpcStatus := s.removeReference(ctx, res.GetShare().GetShare()) + rpcStatus := s.removeReference(ctx, res.GetShare().GetShare().ResourceId) if rpcStatus.Code != rpc.Code_CODE_OK && rpcStatus.Code != rpc.Code_CODE_NOT_FOUND { return &collaboration.UpdateReceivedShareResponse{Status: rpcStatus}, nil } @@ -368,10 +369,10 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update return res, nil } -func (s *svc) removeReference(ctx context.Context, share *collaboration.Share) *rpc.Status { +func (s *svc) removeReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { log := appctx.GetLogger(ctx) - idReference := &provider.Reference{ResourceId: share.ResourceId} + idReference := &provider.Reference{ResourceId: resourceID} storageProvider, err := s.find(ctx, idReference) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { @@ -382,7 +383,7 @@ func (s *svc) removeReference(ctx context.Context, share *collaboration.Share) * statRes, err := storageProvider.Stat(ctx, &provider.StatRequest{Ref: idReference}) if err != nil { - return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+share.ResourceId.String()) + return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) } // FIXME how can we delete a reference if the original resource was deleted? @@ -397,68 +398,51 @@ func (s *svc) removeReference(ctx context.Context, share *collaboration.Share) * return status.NewInternal(ctx, err, "could not delete share reference") } - // We list the shares folder and delete references corresponding to this share - // TODO: We need to maintain a DB of these - shareFolderPath := path.Join(homeRes.Path, s.c.ShareFolder) + sharePath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) + log.Debug().Str("share_path", sharePath).Msg("remove reference of share") - homeProvider, err := s.find(ctx, &provider.Reference{Path: shareFolderPath}) + homeProvider, err := s.find(ctx, &provider.Reference{Path: sharePath}) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { return status.NewNotFound(ctx, "storage provider not found") } return status.NewInternal(ctx, err, "error finding storage provider") } - shareRefs, err := homeProvider.ListContainer(ctx, &provider.ListContainerRequest{ - Ref: &provider.Reference{Path: shareFolderPath}, - }) - if err != nil { - return status.NewInternal(ctx, err, "gateway: error listing shares folder") - } - if shareRefs.Status.Code != rpc.Code_CODE_OK { - err := status.NewErrorFromCode(shareRefs.Status.GetCode(), "gateway") - return status.NewInternal(ctx, err, "could not list shares folder") - } - target := fmt.Sprintf("cs3:%s/%s", share.ResourceId.GetStorageId(), share.ResourceId.GetOpaqueId()) - for _, s := range shareRefs.Infos { - if s.Target == target { - log.Debug().Str("share_path", s.Path).Msg("remove reference of share") + deleteReq := &provider.DeleteRequest{ + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + // This signals the storageprovider that we want to delete the share reference and not the underlying file. + "deleting_shared_resource": {}, + }, + }, + Ref: &provider.Reference{Path: sharePath}, + } - deleteReq := &provider.DeleteRequest{ - Opaque: &typesv1beta1.Opaque{ - Map: map[string]*typesv1beta1.OpaqueEntry{ - // This signals the storageprovider that we want to delete the share reference and not the underlying file. - "deleting_shared_resource": {}, - }, - }, - Ref: &provider.Reference{Path: s.Path}, - } + deleteResp, err := homeProvider.Delete(ctx, deleteReq) + if err != nil { + return status.NewInternal(ctx, err, "could not delete share reference") + } - deleteResp, err := homeProvider.Delete(ctx, deleteReq) - if err != nil { - return status.NewInternal(ctx, err, "could not delete share reference") - } + switch deleteResp.Status.Code { + case rpc.Code_CODE_OK: + // we can continue deleting the reference + case rpc.Code_CODE_NOT_FOUND: + // This is fine, we wanted to delete it anyway + return status.NewOK(ctx) + default: + err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") + return status.NewInternal(ctx, err, "could not delete share reference") + } - switch deleteResp.Status.Code { - case rpc.Code_CODE_OK: - // we can continue deleting the reference - case rpc.Code_CODE_NOT_FOUND: - // This is fine, we wanted to delete it anyway - return status.NewOK(ctx) - default: - err := status.NewErrorFromCode(deleteResp.Status.GetCode(), "gateway") - return status.NewInternal(ctx, err, "could not delete share reference") - } + log.Debug().Str("share_path", sharePath).Msg("share reference successfully removed") - log.Debug().Str("share_path", s.Path).Msg("share reference successfully removed") - } - } return status.NewOK(ctx) } -func (s *svc) createReference(ctx context.Context, share *collaboration.Share) *rpc.Status { +func (s *svc) createReference(ctx context.Context, resourceID *provider.ResourceId) *rpc.Status { ref := &provider.Reference{ - ResourceId: share.ResourceId, + ResourceId: resourceID, } log := appctx.GetLogger(ctx) @@ -477,12 +461,12 @@ func (s *svc) createReference(ctx context.Context, share *collaboration.Share) * statRes, err := c.Stat(ctx, statReq) if err != nil { - return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+share.ResourceId.String()) + return status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id: "+resourceID.String()) } if statRes.Status.Code != rpc.Code_CODE_OK { err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway") - log.Err(err).Msg("gateway: Stat failed on the share resource id: " + share.ResourceId.String()) + log.Err(err).Msg("gateway: Stat failed on the share resource id: " + resourceID.String()) return status.NewInternal(ctx, err, "error updating received share") } @@ -502,8 +486,16 @@ func (s *svc) createReference(ctx context.Context, share *collaboration.Share) * // It is the responsibility of the gateway to resolve these references and merge the response back // from the main request. // TODO(labkode): the name of the share should be the filename it points to by default. - refPath := &provider.Reference{Path: path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path))} - c, err = s.findByPath(ctx, refPath.Path) + refPath := path.Join(homeRes.Path, s.c.ShareFolder, path.Base(statRes.Info.Path)) + log.Info().Msg("mount path will be:" + refPath) + + createRefReq := &provider.CreateReferenceRequest{ + Ref: &provider.Reference{Path: refPath}, + // cs3 is the Scheme and %s/%s is the Opaque parts of a net.URL. + TargetUri: fmt.Sprintf("cs3:%s/%s", resourceID.GetStorageId(), resourceID.GetOpaqueId()), + } + + c, err = s.findByPath(ctx, refPath) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { return status.NewNotFound(ctx, "storage provider not found") @@ -511,24 +503,6 @@ func (s *svc) createReference(ctx context.Context, share *collaboration.Share) * return status.NewInternal(ctx, err, "error finding storage provider") } - refPathStat, err := c.Stat(ctx, &provider.StatRequest{ - Ref: refPath, - }) - if err == nil && refPathStat.Status.Code == rpc.Code_CODE_OK { - // This reference already exists, add extra metadata to avoid conflicts - name := fmt.Sprintf("%s_%s_%s", path.Base(statRes.Info.Path), share.Owner.OpaqueId, utils.TSToTime(share.Ctime).Format("2006-01-02_15-04-05")) - refPath = &provider.Reference{Path: path.Join(homeRes.Path, s.c.ShareFolder, name)} - } - - log.Info().Msgf("refPathStat %+v %+v", refPathStat.Status, err) - log.Info().Msg("mount path will be:" + refPath.Path) - - createRefReq := &provider.CreateReferenceRequest{ - Ref: refPath, - // cs3 is the Scheme and %s/%s is the Opaque parts of a net.URL. - TargetUri: fmt.Sprintf("cs3:%s/%s", share.ResourceId.GetStorageId(), share.ResourceId.GetOpaqueId()), - } - createRefRes, err := c.CreateReference(ctx, createRefReq) if err != nil { log.Err(err).Msg("gateway: error calling GetHome") diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 6d3cadc0f1..e2aa3c0ca5 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -203,12 +203,6 @@ func (s *svc) Handler() http.Handler { head, r.URL.Path = router.ShiftPath(r.URL.Path) log.Debug().Str("head", head).Str("tail", r.URL.Path).Msg("http routing") switch head { - case "cernbox": - var origin string - origin, r.URL.Path = router.ShiftPath(r.URL.Path) // desktop or mobile - _, r.URL.Path = router.ShiftPath(r.URL.Path) // remote.php - head, r.URL.Path = router.ShiftPath(r.URL.Path) - base = path.Join(base, "cernbox", origin, "remote.php") case "status.php": s.doStatus(w, r) return From a8ec504a3b3eb93f55a95cbbf825786fcd3777e2 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 19 Apr 2022 18:08:01 +0200 Subject: [PATCH 7/7] Add GetStorageProviders to uploader scope --- internal/grpc/interceptors/auth/scope.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 7cf59f1d8e..df425f6dbc 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -260,6 +260,10 @@ func extractRefForReaderRole(req interface{}) (*provider.Reference, bool) { func extractRefForUploaderRole(req interface{}) (*provider.Reference, bool) { switch v := req.(type) { // Write Requests + case *registry.GetStorageProvidersRequest: + return v.GetRef(), true + case *provider.StatRequest: + return v.GetRef(), true case *provider.CreateContainerRequest: return v.GetRef(), true case *provider.TouchFileRequest: