Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Resharing #4606

Merged
merged 2 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile.revad-ceph
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ FROM quay.io/ceph/ceph:v18

# replace repo url with one that allows downloading the repo metadata
# if http://download.ceph.com/rpm-reef/el8/x86_64/repodata/repomd.xml works again this can be dropped
RUN sed -i 's/download.ceph.com/de.ceph.com/' /etc/yum.repos.d/ceph.repo
RUN sed -i 's/download.ceph.com/fr.ceph.com/' /etc/yum.repos.d/ceph.repo
RUN mkdir -p /etc/selinux/config

RUN dnf update --exclude=ceph-iscsi,chrony -y && dnf install -y \
Expand Down
5 changes: 5 additions & 0 deletions changelog/unreleased/remove-resharing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: Remove resharing

Removed all code related to resharing

https://github.com/cs3org/reva/pull/4606
4 changes: 2 additions & 2 deletions cmd/reva/ocm-share-create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/reva/share-create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 6 additions & 9 deletions internal/grpc/services/usershareprovider/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -152,41 +152,40 @@ 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,
),
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())
Expand Down Expand Up @@ -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),
)
})
})
Expand Down
3 changes: 2 additions & 1 deletion internal/http/services/owncloud/ocs/data/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ import (

const (
storageIDPrefix string = "shared::"

_resharingDefault bool = false
)

var (
Expand All @@ -92,7 +90,6 @@ type Handler struct {
userIdentifierCache *ttlcache.Cache
statCache cache.StatCache
deniable bool
resharing bool
publicPasswordEnforced passwordEnforced
passwordValidator password.Validator

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 := `<CapabilitiesData><capabilities><files_sharing><api_enabled>1</api_enabled><resharing>0</resharing><group_sharing>0</group_sharing><sharing_roles>0</sharing_roles><deny_access>0</deny_access><auto_accept_share>0</auto_accept_share><share_with_group_members_only>0</share_with_group_members_only><share_with_membership_groups_only>0</share_with_membership_groups_only><search_min_length>0</search_min_length><default_permissions>0</default_permissions></files_sharing></capabilities></CapabilitiesData>`
// 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 := `<CapabilitiesData><capabilities><files_sharing><api_enabled>1</api_enabled><group_sharing>0</group_sharing><sharing_roles>0</sharing_roles><deny_access>0</deny_access><auto_accept_share>0</auto_accept_share><share_with_group_members_only>0</share_with_group_members_only><share_with_membership_groups_only>0</share_with_membership_groups_only><search_min_length>0</search_min_length><default_permissions>0</default_permissions><resharing>0</resharing></files_sharing></capabilities></CapabilitiesData>`

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")
}
}
4 changes: 2 additions & 2 deletions internal/http/services/sciencemesh/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading
Loading