From a00ea2afe2d45c122b6aadf60608bbc5a95a37f2 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Tue, 2 Apr 2024 11:38:13 +0200 Subject: [PATCH] remove resharing Signed-off-by: jkoberg --- changelog/unreleased/remove-resharing.md | 5 ++ cmd/reva/ocm-share-create.go | 4 +- cmd/reva/share-create.go | 4 +- .../ocmshareprovider/ocmshareprovider.go | 2 +- .../publicshareprovider.go | 2 +- .../usershareprovider/usershareprovider.go | 15 ++-- .../usershareprovider_test.go | 35 +++++----- .../owncloud/ocs/data/capabilities.go | 3 +- .../handlers/apps/sharing/shares/shares.go | 23 +++---- .../apps/sharing/shares/shares_test.go | 17 +++-- .../cloud/capabilities/capabilities_test.go | 15 ++-- internal/http/services/sciencemesh/share.go | 4 +- pkg/cbox/utils/conversions.go | 6 +- pkg/conversions/permissions_test.go | 8 +-- pkg/conversions/role.go | 68 +++++++------------ pkg/conversions/role_test.go | 42 ++++++------ .../manager/owncloudsql/owncloudsql_test.go | 4 +- .../utils/decomposedfs/node/node_test.go | 4 +- pkg/storage/utils/eosfs/eosfs.go | 6 +- tests/integration/grpc/ocm_share_test.go | 30 ++------ .../drone/frontend-global.toml | 1 - .../oc-integration-tests/drone/frontend.toml | 1 - .../local-mesh/frontend-global.toml | 1 - .../local-mesh/frontend.toml | 1 - .../oc-integration-tests/local/combined.toml | 1 - .../local/frontend-global.toml | 1 - .../oc-integration-tests/local/frontend.toml | 1 - 27 files changed, 131 insertions(+), 173 deletions(-) create mode 100644 changelog/unreleased/remove-resharing.md diff --git a/changelog/unreleased/remove-resharing.md b/changelog/unreleased/remove-resharing.md new file mode 100644 index 0000000000..2fd1e25a95 --- /dev/null +++ b/changelog/unreleased/remove-resharing.md @@ -0,0 +1,5 @@ +Enhancement: Remove resharing + +Removed all code related to resharing + +https://github.com/cs3org/reva/pull/4606 diff --git a/cmd/reva/ocm-share-create.go b/cmd/reva/ocm-share-create.go index 65ce3f9088..13135b666f 100644 --- a/cmd/reva/ocm-share-create.go +++ b/cmd/reva/ocm-share-create.go @@ -186,9 +186,9 @@ func getAccessMethods(webdav, webapp, datatx bool, rol string) ([]*ocm.AccessMet func getOCMSharePerm(p string) (*provider.ResourcePermissions, error) { switch p { case viewerPermission: - return conversions.NewViewerRole(false).CS3ResourcePermissions(), nil + return conversions.NewViewerRole().CS3ResourcePermissions(), nil case editorPermission: - return conversions.NewEditorRole(false).CS3ResourcePermissions(), nil + return conversions.NewEditorRole().CS3ResourcePermissions(), nil } return nil, errors.New("invalid rol: " + p) } diff --git a/cmd/reva/share-create.go b/cmd/reva/share-create.go index 5579c3b631..5c6172cc74 100644 --- a/cmd/reva/share-create.go +++ b/cmd/reva/share-create.go @@ -159,9 +159,9 @@ func getGrantType(t string) provider.GranteeType { func getSharePerm(p string) (*provider.ResourcePermissions, error) { switch p { case viewerPermission: - return conversions.NewViewerRole(false).CS3ResourcePermissions(), nil + return conversions.NewViewerRole().CS3ResourcePermissions(), nil case editorPermission: - return conversions.NewEditorRole(false).CS3ResourcePermissions(), nil + return conversions.NewEditorRole().CS3ResourcePermissions(), nil case collabPermission: return conversions.NewCoownerRole().CS3ResourcePermissions(), nil case denyPermission: diff --git a/internal/grpc/services/ocmshareprovider/ocmshareprovider.go b/internal/grpc/services/ocmshareprovider/ocmshareprovider.go index 0f68136fdf..5c5824540c 100644 --- a/internal/grpc/services/ocmshareprovider/ocmshareprovider.go +++ b/internal/grpc/services/ocmshareprovider/ocmshareprovider.go @@ -327,7 +327,7 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq ProviderID: ocmshare.Id.OpaqueId, Owner: formatOCMUser(&userpb.UserId{ OpaqueId: info.Owner.OpaqueId, - Idp: s.conf.ProviderDomain, // FIXME: this is not generally true in case of resharing + Idp: s.conf.ProviderDomain, }), Sender: formatOCMUser(&userpb.UserId{ OpaqueId: user.Id.OpaqueId, diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index 3df749f882..b987b96acf 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -593,7 +593,7 @@ func isInternalLink(req *link.UpdatePublicShareRequest, ps *link.PublicShare) bo } func enforcePassword(canOptOut bool, permissions *provider.ResourcePermissions, conf *config) bool { - isReadOnly := conversions.SufficientCS3Permissions(conversions.NewViewerRole(true).CS3ResourcePermissions(), permissions) + isReadOnly := conversions.SufficientCS3Permissions(conversions.NewViewerRole().CS3ResourcePermissions(), permissions) if isReadOnly && canOptOut { return false } diff --git a/internal/grpc/services/usershareprovider/usershareprovider.go b/internal/grpc/services/usershareprovider/usershareprovider.go index 0c503e7cc2..779409faac 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider.go +++ b/internal/grpc/services/usershareprovider/usershareprovider.go @@ -55,7 +55,6 @@ type config struct { Drivers map[string]map[string]interface{} `mapstructure:"drivers"` GatewayAddr string `mapstructure:"gateway_addr"` AllowedPathsForShares []string `mapstructure:"allowed_paths_for_shares"` - DisableResharing bool `mapstructure:"disable_resharing"` } func (c *config) init() { @@ -68,7 +67,6 @@ type service struct { sm share.Manager gatewaySelector pool.Selectable[gateway.GatewayAPIClient] allowedPathsForShares []*regexp.Regexp - disableResharing bool } func getShareManager(c *config) (share.Manager, error) { @@ -129,16 +127,15 @@ func NewDefault(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error return nil, err } - return New(gatewaySelector, sm, allowedPathsForShares, c.DisableResharing), nil + return New(gatewaySelector, sm, allowedPathsForShares), nil } // New creates a new user share provider svc -func New(gatewaySelector pool.Selectable[gateway.GatewayAPIClient], sm share.Manager, allowedPathsForShares []*regexp.Regexp, disableResharing bool) rgrpc.Service { +func New(gatewaySelector pool.Selectable[gateway.GatewayAPIClient], sm share.Manager, allowedPathsForShares []*regexp.Regexp) rgrpc.Service { service := &service{ sm: sm, gatewaySelector: gatewaySelector, allowedPathsForShares: allowedPathsForShares, - disableResharing: disableResharing, } return service @@ -160,8 +157,8 @@ func (s *service) CreateShare(ctx context.Context, req *collaboration.CreateShar log := appctx.GetLogger(ctx) user := ctxpkg.ContextMustGetUser(ctx) - // when resharing is disabled grants must not allow grant permissions - if s.disableResharing && HasGrantPermissions(req.GetGrant().GetPermissions().GetPermissions()) { + // Grants must not allow grant permissions + if HasGrantPermissions(req.GetGrant().GetPermissions().GetPermissions()) { return &collaboration.CreateShareResponse{ Status: status.NewInvalidArg(ctx, "resharing not supported"), }, nil @@ -342,8 +339,8 @@ func (s *service) UpdateShare(ctx context.Context, req *collaboration.UpdateShar log := appctx.GetLogger(ctx) user := ctxpkg.ContextMustGetUser(ctx) - // when resharing is disabled grants must not allow grant permissions - if s.disableResharing && HasGrantPermissions(req.GetShare().GetPermissions().GetPermissions()) { + // Grants must not allow grant permissions + if HasGrantPermissions(req.GetShare().GetPermissions().GetPermissions()) { return &collaboration.UpdateShareResponse{ Status: status.NewInvalidArg(ctx, "resharing not supported"), }, nil diff --git a/internal/grpc/services/usershareprovider/usershareprovider_test.go b/internal/grpc/services/usershareprovider/usershareprovider_test.go index 7b958eda4c..6e2087a890 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider_test.go +++ b/internal/grpc/services/usershareprovider/usershareprovider_test.go @@ -58,7 +58,7 @@ var _ = Describe("user share provider service", func() { cs3permissionsNoAddGrant *providerpb.ResourcePermissions getShareResponse *collaborationpb.Share ) - cs3permissionsNoAddGrant = conversions.RoleFromName("manager", true).CS3ResourcePermissions() + cs3permissionsNoAddGrant = conversions.RoleFromName("manager").CS3ResourcePermissions() cs3permissionsNoAddGrant.AddGrant = false BeforeEach(func() { @@ -112,7 +112,7 @@ var _ = Describe("user share provider service", func() { } manager.On("GetShare", mock.Anything, mock.Anything).Return(getShareResponse, nil) - rgrpcService := usershareprovider.New(gatewaySelector, manager, []*regexp.Regexp{}, false) + rgrpcService := usershareprovider.New(gatewaySelector, manager, []*regexp.Regexp{}) provider = rgrpcService.(collaborationpb.CollaborationAPIServer) Expect(provider).ToNot(BeNil()) @@ -152,16 +152,16 @@ var _ = Describe("user share provider service", func() { }, Entry( "insufficient permissions", - conversions.RoleFromName("spaceeditor", true).CS3ResourcePermissions(), - conversions.RoleFromName("manager", true).CS3ResourcePermissions(), + conversions.RoleFromName("spaceeditor").CS3ResourcePermissions(), + conversions.RoleFromName("manager").CS3ResourcePermissions(), rpcpb.Code_CODE_OK, - rpcpb.Code_CODE_PERMISSION_DENIED, + rpcpb.Code_CODE_INVALID_ARGUMENT, 0, ), Entry( "sufficient permissions", - conversions.RoleFromName("manager", true).CS3ResourcePermissions(), - conversions.RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + conversions.RoleFromName("manager").CS3ResourcePermissions(), + conversions.RoleFromName("spaceeditor").CS3ResourcePermissions(), rpcpb.Code_CODE_OK, rpcpb.Code_CODE_OK, 1, @@ -169,24 +169,23 @@ var _ = Describe("user share provider service", func() { Entry( "no AddGrant permission on resource", cs3permissionsNoAddGrant, - conversions.RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + conversions.RoleFromName("spaceeditor").CS3ResourcePermissions(), rpcpb.Code_CODE_OK, rpcpb.Code_CODE_PERMISSION_DENIED, 0, ), Entry( "no WriteShare permission on user role", - conversions.RoleFromName("manager", true).CS3ResourcePermissions(), - conversions.RoleFromName("mspaceeditor", true).CS3ResourcePermissions(), + conversions.RoleFromName("manager").CS3ResourcePermissions(), + conversions.RoleFromName("mspaceeditor").CS3ResourcePermissions(), rpcpb.Code_CODE_PERMISSION_DENIED, rpcpb.Code_CODE_PERMISSION_DENIED, 0, ), ) - Context("resharing disabled", func() { + Context("resharing is not allowed", func() { JustBeforeEach(func() { - // disable resharing - rgrpcService := usershareprovider.New(gatewaySelector, manager, []*regexp.Regexp{}, true) + rgrpcService := usershareprovider.New(gatewaySelector, manager, []*regexp.Regexp{}) provider = rgrpcService.(collaborationpb.CollaborationAPIServer) Expect(provider).ToNot(BeNil()) @@ -222,11 +221,11 @@ var _ = Describe("user share provider service", func() { manager.AssertNumberOfCalls(GinkgoT(), "Share", expectedCalls) }, - Entry("AddGrant", conversions.RoleFromName("manager", true).CS3ResourcePermissions(), &providerpb.ResourcePermissions{AddGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), - Entry("UpdateGrant", conversions.RoleFromName("manager", true).CS3ResourcePermissions(), &providerpb.ResourcePermissions{UpdateGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), - Entry("RemoveGrant", conversions.RoleFromName("manager", true).CS3ResourcePermissions(), &providerpb.ResourcePermissions{RemoveGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), - Entry("DenyGrant", conversions.RoleFromName("manager", true).CS3ResourcePermissions(), &providerpb.ResourcePermissions{DenyGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), - Entry("ListGrants", conversions.RoleFromName("manager", true).CS3ResourcePermissions(), &providerpb.ResourcePermissions{ListGrants: true}, rpcpb.Code_CODE_OK, 1), + Entry("AddGrant", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{AddGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), + Entry("UpdateGrant", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{UpdateGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), + Entry("RemoveGrant", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{RemoveGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), + Entry("DenyGrant", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{DenyGrant: true}, rpcpb.Code_CODE_INVALID_ARGUMENT, 0), + Entry("ListGrants", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{ListGrants: true}, rpcpb.Code_CODE_OK, 1), ) }) }) diff --git a/internal/http/services/owncloud/ocs/data/capabilities.go b/internal/http/services/owncloud/ocs/data/capabilities.go index 369ca23179..82e2fdf2b9 100644 --- a/internal/http/services/owncloud/ocs/data/capabilities.go +++ b/internal/http/services/owncloud/ocs/data/capabilities.go @@ -209,7 +209,6 @@ type CapabilitiesDav struct { // CapabilitiesFilesSharing TODO document type CapabilitiesFilesSharing struct { APIEnabled ocsBool `json:"api_enabled" xml:"api_enabled" mapstructure:"api_enabled"` - Resharing ocsBool `json:"resharing" xml:"resharing"` GroupSharing ocsBool `json:"group_sharing" xml:"group_sharing" mapstructure:"group_sharing"` SharingRoles ocsBool `json:"sharing_roles" xml:"sharing_roles" mapstructure:"sharing_roles"` DenyAccess ocsBool `json:"deny_access" xml:"deny_access" mapstructure:"deny_access"` @@ -222,6 +221,8 @@ type CapabilitiesFilesSharing struct { Federation *CapabilitiesFilesSharingFederation `json:"federation" xml:"federation"` Public *CapabilitiesFilesSharingPublic `json:"public" xml:"public"` User *CapabilitiesFilesSharingUser `json:"user" xml:"user"` + // TODO: Remove next line once web defaults to resharing=false + Resharing ocsBool `json:"resharing" xml:"resharing"` } // CapabilitiesFilesSharingPublic TODO document diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 694273866e..4a46ebaa13 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -69,8 +69,6 @@ import ( const ( storageIDPrefix string = "shared::" - - _resharingDefault bool = false ) var ( @@ -92,7 +90,6 @@ type Handler struct { userIdentifierCache *ttlcache.Cache statCache cache.StatCache deniable bool - resharing bool publicPasswordEnforced passwordEnforced passwordValidator password.Validator @@ -146,7 +143,6 @@ func (h *Handler) Init(c *config.Config) error { h.userIdentifierCache = ttlcache.NewCache() _ = h.userIdentifierCache.SetTTL(time.Second * time.Duration(c.UserIdentifierCacheTTL)) h.deniable = c.EnableDenials - h.resharing = resharing(c) h.publicPasswordEnforced = publicPwdEnforced(c) h.passwordValidator = passwordPolicies(c) @@ -292,6 +288,12 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { return } + // resharing is forbidden + if role.CS3ResourcePermissions().GetAddGrant() { + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, "resharing not supported", nil) + return + } + var share *collaboration.Share if shareType == int(conversions.ShareTypeUser) { share, ocsErr = h.createUserShare(w, r, statRes.Info, role, val) @@ -320,7 +322,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { response.WriteOCSSuccess(w, r, s) case int(conversions.ShareTypePublicLink): // public links default to read only - _, _, ocsErr := h.extractPermissions(reqRole, reqPermissions, statRes.Info, conversions.NewViewerRole(h.resharing)) + _, _, ocsErr := h.extractPermissions(reqRole, reqPermissions, statRes.Info, conversions.NewViewerRole()) if ocsErr != nil && ocsErr.Error != conversions.ErrZeroPermission { response.WriteOCSError(w, r, http.StatusForbidden, "No share permission", nil) return @@ -339,7 +341,7 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { response.WriteOCSSuccess(w, r, s) case int(conversions.ShareTypeFederatedCloudShare): // federated shares default to read only - if role, val, err := h.extractPermissions(reqRole, reqPermissions, statRes.Info, conversions.NewViewerRole(h.resharing)); err == nil { + if role, val, err := h.extractPermissions(reqRole, reqPermissions, statRes.Info, conversions.NewViewerRole()); err == nil { h.createFederatedCloudShare(w, r, statRes.Info, role, val) } case int(conversions.ShareTypeSpaceMembershipUser), int(conversions.ShareTypeSpaceMembershipGroup): @@ -429,7 +431,7 @@ func (h *Handler) extractPermissions(reqRole string, reqPermissions string, ri * // the share role overrides the requested permissions if reqRole != "" { - role = conversions.RoleFromName(reqRole, h.resharing) + role = conversions.RoleFromName(reqRole) } // if the role is unknown - fall back to reqPermissions or defaultPermissions @@ -1685,10 +1687,3 @@ func sufficientPermissions(existing, requested *provider.ResourcePermissions, is rp := conversions.RoleFromResourcePermissions(requested, islink).OCSPermissions() return ep.Contain(rp) } - -func resharing(c *config.Config) bool { - if c != nil && c.Capabilities.Capabilities != nil && c.Capabilities.Capabilities.FilesSharing != nil { - return bool(c.Capabilities.Capabilities.FilesSharing.Resharing) - } - return _resharingDefault -} diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go index cc8c83ce5b..9ef0416083 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go @@ -272,10 +272,15 @@ var _ = Describe("The ocs API", func() { Id: resID, Owner: user.Id, PermissionSet: &provider.ResourcePermissions{ - Stat: true, - AddGrant: true, - UpdateGrant: true, - RemoveGrant: true, + Stat: true, + ListContainer: true, + AddGrant: true, + UpdateGrant: true, + RemoveGrant: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + ListRecycle: true, }, Size: 10, }, @@ -311,7 +316,7 @@ var _ = Describe("The ocs API", func() { form.Add("shareType", "0") form.Add("path", "/newshare") form.Add("name", "newshare") - form.Add("permissions", "16") + form.Add("permissions", "1") form.Add("shareWith", "admin") req := httptest.NewRequest("POST", "/apps/files_sharing/api/v1/shares", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") @@ -362,7 +367,7 @@ var _ = Describe("The ocs API", func() { form.Add("shareType", "0") form.Add("path", "/newshare") form.Add("name", "newshare") - form.Add("permissions", "16") + form.Add("permissions", "1") form.Add("shareWith", "admin") req := httptest.NewRequest("POST", "/apps/files_sharing/api/v1/shares", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go index 43ae2afcdc..d8202f2965 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities_test.go @@ -35,24 +35,27 @@ func TestMarshal(t *testing.T) { }, } - jsonExpect := `{"capabilities":{"core":null,"checksums":null,"files":null,"dav":null,"files_sharing":{"api_enabled":true,"resharing":false,"group_sharing":false,"sharing_roles":false,"deny_access":false,"auto_accept_share":false,"share_with_group_members_only":false,"share_with_membership_groups_only":false,"search_min_length":0,"default_permissions":0,"user_enumeration":null,"federation":null,"public":null,"user":null}},"version":null}` - xmlExpect := `1000000000` + // TODO: remove resharing from these strings once web defaults to resharing=false + jsonExpect := `{"capabilities":{"core":null,"checksums":null,"files":null,"dav":null,"files_sharing":{"api_enabled":true,"group_sharing":false,"sharing_roles":false,"deny_access":false,"auto_accept_share":false,"share_with_group_members_only":false,"share_with_membership_groups_only":false,"search_min_length":0,"default_permissions":0,"user_enumeration":null,"federation":null,"public":null,"user":null,"resharing":false}},"version":null}` + xmlExpect := `1000000000` jsonData, err := json.Marshal(&cd) if err != nil { - t.Fail() + t.Fatal("cant marshal json") } if string(jsonData) != jsonExpect { - t.Fail() + t.Log(string(jsonData)) + t.Fatal("json data does not match") } xmlData, err := xml.Marshal(&cd) if err != nil { - t.Fail() + t.Fatal("cant marshal xml") } if string(xmlData) != xmlExpect { - t.Fail() + t.Log(string(xmlData)) + t.Fatal("xml data does not match") } } diff --git a/internal/http/services/sciencemesh/share.go b/internal/http/services/sciencemesh/share.go index af51c5e6b4..c5bc78e562 100644 --- a/internal/http/services/sciencemesh/share.go +++ b/internal/http/services/sciencemesh/share.go @@ -150,9 +150,9 @@ func (h *sharesHandler) CreateShare(w http.ResponseWriter, r *http.Request) { func getPermissionsByRole(role string) (*providerpb.ResourcePermissions, appprovider.ViewMode) { switch role { case "viewer": - return conversions.NewViewerRole(false).CS3ResourcePermissions(), appprovider.ViewMode_VIEW_MODE_READ_ONLY + return conversions.NewViewerRole().CS3ResourcePermissions(), appprovider.ViewMode_VIEW_MODE_READ_ONLY case "editor": - return conversions.NewEditorRole(false).CS3ResourcePermissions(), appprovider.ViewMode_VIEW_MODE_READ_WRITE + return conversions.NewEditorRole().CS3ResourcePermissions(), appprovider.ViewMode_VIEW_MODE_READ_WRITE } return nil, 0 } diff --git a/pkg/cbox/utils/conversions.go b/pkg/cbox/utils/conversions.go index a2cc7d1818..b94b50b3b9 100644 --- a/pkg/cbox/utils/conversions.go +++ b/pkg/cbox/utils/conversions.go @@ -145,12 +145,12 @@ func SharePermToInt(p *provider.ResourcePermissions) int { func IntTosharePerm(p int, itemType string) *provider.ResourcePermissions { switch p { case 1: - return conversions.NewViewerRole(false).CS3ResourcePermissions() + return conversions.NewViewerRole().CS3ResourcePermissions() case 15: if itemType == "folder" { - return conversions.NewEditorRole(false).CS3ResourcePermissions() + return conversions.NewEditorRole().CS3ResourcePermissions() } - return conversions.NewFileEditorRole(false).CS3ResourcePermissions() + return conversions.NewFileEditorRole().CS3ResourcePermissions() case 4: return conversions.NewUploaderRole().CS3ResourcePermissions() default: diff --git a/pkg/conversions/permissions_test.go b/pkg/conversions/permissions_test.go index 39087ee87d..6aae001e5d 100644 --- a/pkg/conversions/permissions_test.go +++ b/pkg/conversions/permissions_test.go @@ -182,23 +182,19 @@ func TestPermissions2Role(t *testing.T) { table := []permissionOnResourceInfo2Role{ { - permissions: PermissionRead | PermissionShare, + permissions: PermissionRead, role: RoleViewer, resourceInfo: resourceInfoDir, }, { permissions: PermissionRead | PermissionShare, role: RoleLegacy, resourceInfo: resourceInfoSpaceRoot, - }, { - permissions: PermissionRead, - role: RoleLegacy, - resourceInfo: resourceInfoDir, }, { permissions: PermissionRead, role: RoleSpaceViewer, resourceInfo: resourceInfoSpaceRoot, }, { - permissions: PermissionRead | PermissionWrite | PermissionCreate | PermissionDelete | PermissionShare, + permissions: PermissionRead | PermissionWrite | PermissionCreate | PermissionDelete, role: RoleEditor, resourceInfo: nil, }, { diff --git a/pkg/conversions/role.go b/pkg/conversions/role.go index cbde91bffc..29621c4aae 100644 --- a/pkg/conversions/role.go +++ b/pkg/conversions/role.go @@ -141,20 +141,20 @@ func (r *Role) WebDAVPermissions(isDir, isShared, isMountpoint, isPublic bool) s } // RoleFromName creates a role from the name -func RoleFromName(name string, sharing bool) *Role { +func RoleFromName(name string) *Role { switch name { case RoleDenied: return NewDeniedRole() case RoleViewer: - return NewViewerRole(sharing) + return NewViewerRole() case RoleSpaceViewer: return NewSpaceViewerRole() case RoleEditor: - return NewEditorRole(sharing) + return NewEditorRole() case RoleSpaceEditor: return NewSpaceEditorRole() case RoleFileEditor: - return NewFileEditorRole(sharing) + return NewFileEditorRole() case RoleUploader: return NewUploaderRole() case RoleManager: @@ -183,15 +183,11 @@ func NewDeniedRole() *Role { } // NewViewerRole creates a viewer role. `sharing` indicates if sharing permission should be added -func NewViewerRole(sharing bool) *Role { +func NewViewerRole() *Role { p := PermissionRead - if sharing { - p |= PermissionShare - } return &Role{ Name: RoleViewer, cS3ResourcePermissions: &provider.ResourcePermissions{ - AddGrant: sharing, GetPath: true, GetQuota: true, InitiateFileDownload: true, @@ -221,15 +217,11 @@ func NewSpaceViewerRole() *Role { } // NewEditorRole creates an editor role. `sharing` indicates if sharing permission should be added -func NewEditorRole(sharing bool) *Role { +func NewEditorRole() *Role { p := PermissionRead | PermissionCreate | PermissionWrite | PermissionDelete - if sharing { - p |= PermissionShare - } return &Role{ Name: RoleEditor, cS3ResourcePermissions: &provider.ResourcePermissions{ - AddGrant: sharing, CreateContainer: true, Delete: true, GetPath: true, @@ -271,15 +263,11 @@ func NewSpaceEditorRole() *Role { } // NewFileEditorRole creates a file-editor role -func NewFileEditorRole(sharing bool) *Role { +func NewFileEditorRole() *Role { p := PermissionRead | PermissionWrite - if sharing { - p |= PermissionShare - } return &Role{ Name: RoleEditor, cS3ResourcePermissions: &provider.ResourcePermissions{ - AddGrant: sharing, GetPath: true, GetQuota: true, InitiateFileDownload: true, @@ -378,34 +366,30 @@ func NewManagerRole() *Role { // RoleFromOCSPermissions tries to map ocs permissions to a role // TODO: rethink using this. ocs permissions cannot be assigned 1:1 to roles func RoleFromOCSPermissions(p Permissions, ri *provider.ResourceInfo) *Role { - if p == PermissionInvalid { + switch { + // Invalid + case p == PermissionInvalid: return NewNoneRole() - } - - if p.Contain(PermissionRead) { - if p.Contain(PermissionWrite) && p.Contain(PermissionCreate) && p.Contain(PermissionDelete) { - if p.Contain(PermissionShare) { - return NewEditorRole(true) - } - - if isSpaceRoot(ri) { - return NewSpaceEditorRole() - } - } - - if p == PermissionRead && isSpaceRoot(ri) { + // Uploader + case p == PermissionCreate: + return NewUploaderRole() + // Viewer/SpaceViewer + case p == PermissionRead: + if isSpaceRoot(ri) { return NewSpaceViewerRole() } - - if p == PermissionRead|PermissionShare && !isSpaceRoot(ri) { - return NewViewerRole(true) + return NewViewerRole() + // Editor/SpaceEditor + case p.Contain(PermissionRead) && p.Contain(PermissionWrite) && p.Contain(PermissionCreate) && p.Contain(PermissionDelete) && !p.Contain(PermissionShare): + if isSpaceRoot(ri) { + return NewSpaceEditorRole() } + + return NewEditorRole() + // Custom + default: + return NewLegacyRoleFromOCSPermissions(p) } - if p == PermissionCreate { - return NewUploaderRole() - } - // legacy - return NewLegacyRoleFromOCSPermissions(p) } func isSpaceRoot(ri *provider.ResourceInfo) bool { diff --git a/pkg/conversions/role_test.go b/pkg/conversions/role_test.go index 44c8499fa3..0246cae22b 100644 --- a/pkg/conversions/role_test.go +++ b/pkg/conversions/role_test.go @@ -20,58 +20,58 @@ func TestSufficientPermissions(t *testing.T) { Sufficient: false, }, { - Existing: RoleFromName("editor", true).CS3ResourcePermissions(), + Existing: RoleFromName("editor").CS3ResourcePermissions(), Requested: nil, Sufficient: false, }, { Existing: nil, - Requested: RoleFromName("viewer", true).CS3ResourcePermissions(), + Requested: RoleFromName("viewer").CS3ResourcePermissions(), Sufficient: false, }, { - Existing: RoleFromName("editor", true).CS3ResourcePermissions(), - Requested: RoleFromName("viewer", true).CS3ResourcePermissions(), + Existing: RoleFromName("editor").CS3ResourcePermissions(), + Requested: RoleFromName("viewer").CS3ResourcePermissions(), Sufficient: true, }, { - Existing: RoleFromName("viewer", true).CS3ResourcePermissions(), - Requested: RoleFromName("editor", true).CS3ResourcePermissions(), + Existing: RoleFromName("viewer").CS3ResourcePermissions(), + Requested: RoleFromName("editor").CS3ResourcePermissions(), Sufficient: false, }, { - Existing: RoleFromName("spaceviewer", true).CS3ResourcePermissions(), - Requested: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + Existing: RoleFromName("spaceviewer").CS3ResourcePermissions(), + Requested: RoleFromName("spaceeditor").CS3ResourcePermissions(), Sufficient: false, }, { - Existing: RoleFromName("manager", true).CS3ResourcePermissions(), - Requested: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + Existing: RoleFromName("manager").CS3ResourcePermissions(), + Requested: RoleFromName("spaceeditor").CS3ResourcePermissions(), Sufficient: true, }, { - Existing: RoleFromName("manager", true).CS3ResourcePermissions(), - Requested: RoleFromName("spaceviewer", true).CS3ResourcePermissions(), + Existing: RoleFromName("manager").CS3ResourcePermissions(), + Requested: RoleFromName("spaceviewer").CS3ResourcePermissions(), Sufficient: true, }, { - Existing: RoleFromName("manager", true).CS3ResourcePermissions(), - Requested: RoleFromName("manager", true).CS3ResourcePermissions(), + Existing: RoleFromName("manager").CS3ResourcePermissions(), + Requested: RoleFromName("manager").CS3ResourcePermissions(), Sufficient: true, }, { - Existing: RoleFromName("manager", true).CS3ResourcePermissions(), - Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Existing: RoleFromName("manager").CS3ResourcePermissions(), + Requested: RoleFromName("denied").CS3ResourcePermissions(), Sufficient: true, }, { - Existing: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), - Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Existing: RoleFromName("spaceeditor").CS3ResourcePermissions(), + Requested: RoleFromName("denied").CS3ResourcePermissions(), Sufficient: false, }, { - Existing: RoleFromName("editor", true).CS3ResourcePermissions(), - Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Existing: RoleFromName("editor").CS3ResourcePermissions(), + Requested: RoleFromName("denied").CS3ResourcePermissions(), Sufficient: false, }, { @@ -97,7 +97,7 @@ func TestSufficientPermissions(t *testing.T) { UpdateGrant: true, DenyGrant: true, }, - Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Requested: RoleFromName("denied").CS3ResourcePermissions(), Sufficient: true, }, } diff --git a/pkg/publicshare/manager/owncloudsql/owncloudsql_test.go b/pkg/publicshare/manager/owncloudsql/owncloudsql_test.go index d3aa8ace36..6822800bb7 100644 --- a/pkg/publicshare/manager/owncloudsql/owncloudsql_test.go +++ b/pkg/publicshare/manager/owncloudsql/owncloudsql_test.go @@ -110,7 +110,7 @@ var _ = Describe("SQL manager", func() { } grant = &link.Grant{ Permissions: &link.PublicSharePermissions{ - Permissions: conversions.NewViewerRole(true).CS3ResourcePermissions(), + Permissions: conversions.NewViewerRole().CS3ResourcePermissions(), }, } }) @@ -449,7 +449,7 @@ var _ = Describe("SQL manager", func() { Ref: ref, Update: &link.UpdatePublicShareRequest_Update{ Type: link.UpdatePublicShareRequest_Update_TYPE_PERMISSIONS, - Grant: &link.Grant{Permissions: &link.PublicSharePermissions{Permissions: conversions.NewEditorRole(false).CS3ResourcePermissions()}}, + Grant: &link.Grant{Permissions: &link.PublicSharePermissions{Permissions: conversions.NewEditorRole().CS3ResourcePermissions()}}, }, }) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/storage/utils/decomposedfs/node/node_test.go b/pkg/storage/utils/decomposedfs/node/node_test.go index 8245438903..06e2c7243e 100644 --- a/pkg/storage/utils/decomposedfs/node/node_test.go +++ b/pkg/storage/utils/decomposedfs/node/node_test.go @@ -285,13 +285,13 @@ var _ = Describe("Node", func() { UserId: u.Id, }, }, - Permissions: ocsconv.NewEditorRole(false).CS3ResourcePermissions(), + Permissions: ocsconv.NewEditorRole().CS3ResourcePermissions(), }) Expect(err).ToNot(HaveOccurred()) spaceRoot, err := env.Lookup.NodeFromSpaceID(env.Ctx, storageSpace.SpaceId) Expect(err).ToNot(HaveOccurred()) permissionsActual, _ := spaceRoot.PermissionSet(env.Ctx) - permissionsExpected := ocsconv.NewEditorRole(false).CS3ResourcePermissions() + permissionsExpected := ocsconv.NewEditorRole().CS3ResourcePermissions() Expect(grants.PermissionsEqual(&permissionsActual, permissionsExpected)).To(BeTrue()) env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything, mock.Anything).Return(provider.ResourcePermissions{ Stat: true, diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 50f9493f80..2d11ecdfa7 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -83,8 +83,6 @@ const LockTypeKey = "reva.lock.type" var hiddenReg = regexp.MustCompile(`\.sys\..#.`) -var _resharing = false - func (c *Config) init() { c.Namespace = path.Clean(c.Namespace) if !strings.HasPrefix(c.Namespace, "/") { @@ -2129,12 +2127,12 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI // The role names should not be hardcoded any more as they will come from config in the future if publicShare, ok := u.Opaque.Map["public-share-role"]; ok { if string(publicShare.Value) == "editor" { - return conversions.NewEditorRole(_resharing).CS3ResourcePermissions() + return conversions.NewEditorRole().CS3ResourcePermissions() } else if string(publicShare.Value) == "uploader" { return conversions.NewUploaderRole().CS3ResourcePermissions() } // Default to viewer role - return conversions.NewViewerRole(_resharing).CS3ResourcePermissions() + return conversions.NewViewerRole().CS3ResourcePermissions() } } diff --git a/tests/integration/grpc/ocm_share_test.go b/tests/integration/grpc/ocm_share_test.go index 7c21334d92..2474a1633f 100644 --- a/tests/integration/grpc/ocm_share_test.go +++ b/tests/integration/grpc/ocm_share_test.go @@ -234,7 +234,7 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewViewerRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewViewerRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) @@ -316,7 +316,7 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewEditorRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewEditorRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) @@ -413,7 +413,7 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewViewerRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewViewerRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) @@ -525,7 +525,7 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewEditorRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewEditorRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) @@ -681,30 +681,12 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewEditorRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewEditorRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) Expect(err).ToNot(HaveOccurred()) Expect(createShareRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK)) - - By("resharing the same file with marie") - - createShareRes2, err := cernboxgw.CreateOCMShare(ctxEinstein, &ocmv1beta1.CreateOCMShareRequest{ - ResourceId: info.Id, - Grantee: &provider.Grantee{ - Type: provider.GranteeType_GRANTEE_TYPE_USER, - Id: &provider.Grantee_UserId{ - UserId: federatedMarieID, - }, - }, - AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewEditorRole(false).CS3ResourcePermissions()), - }, - RecipientMeshProvider: cesnet.ProviderInfo, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(createShareRes2.Status.Code).To(Equal(rpcv1beta1.Code_CODE_ALREADY_EXISTS)) }) }) @@ -725,7 +707,7 @@ var _ = Describe("ocm share", func() { }, }, AccessMethods: []*ocmv1beta1.AccessMethod{ - share.NewWebDavAccessMethod(conversions.NewEditorRole(false).CS3ResourcePermissions()), + share.NewWebDavAccessMethod(conversions.NewEditorRole().CS3ResourcePermissions()), }, RecipientMeshProvider: cesnet.ProviderInfo, }) diff --git a/tests/oc-integration-tests/drone/frontend-global.toml b/tests/oc-integration-tests/drone/frontend-global.toml index 6fd2e2bc72..b7b7e6db8a 100644 --- a/tests/oc-integration-tests/drone/frontend-global.toml +++ b/tests/oc-integration-tests/drone/frontend-global.toml @@ -61,7 +61,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/drone/frontend.toml b/tests/oc-integration-tests/drone/frontend.toml index 07679cf736..fcfd6d25fe 100644 --- a/tests/oc-integration-tests/drone/frontend.toml +++ b/tests/oc-integration-tests/drone/frontend.toml @@ -62,7 +62,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/local-mesh/frontend-global.toml b/tests/oc-integration-tests/local-mesh/frontend-global.toml index a98f7e31b2..ca51f4c3c8 100644 --- a/tests/oc-integration-tests/local-mesh/frontend-global.toml +++ b/tests/oc-integration-tests/local-mesh/frontend-global.toml @@ -55,7 +55,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/local-mesh/frontend.toml b/tests/oc-integration-tests/local-mesh/frontend.toml index 14a7259bd4..87436854ca 100644 --- a/tests/oc-integration-tests/local-mesh/frontend.toml +++ b/tests/oc-integration-tests/local-mesh/frontend.toml @@ -57,7 +57,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/local/combined.toml b/tests/oc-integration-tests/local/combined.toml index 4b0262e6a7..264c74bb32 100644 --- a/tests/oc-integration-tests/local/combined.toml +++ b/tests/oc-integration-tests/local/combined.toml @@ -136,7 +136,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/local/frontend-global.toml b/tests/oc-integration-tests/local/frontend-global.toml index 2c8c1a254c..d9b7c31947 100644 --- a/tests/oc-integration-tests/local/frontend-global.toml +++ b/tests/oc-integration-tests/local/frontend-global.toml @@ -62,7 +62,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true diff --git a/tests/oc-integration-tests/local/frontend.toml b/tests/oc-integration-tests/local/frontend.toml index 86071204f9..a6d4a9786e 100644 --- a/tests/oc-integration-tests/local/frontend.toml +++ b/tests/oc-integration-tests/local/frontend.toml @@ -70,7 +70,6 @@ versionstring = "10.0.11" [http.services.ocs.capabilities.capabilities.files_sharing] api_enabled = true -resharing = true group_sharing = true auto_accept_share = true share_with_group_members_only = true