Skip to content

Commit

Permalink
Merge pull request #3849 from aduffeck/prevent-sharing-space-roots
Browse files Browse the repository at this point in the history 
Prevent sharing space roots
  • Loading branch information
@aduffeck
aduffeck authored May 10, 2023
2 parents 4f9d9da + 499fbf0 commit 98d8707
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 119 deletions.
6 changes: 3 additions & 3 deletions .drone.env
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# The test runner source for API tests
APITESTS_COMMITID=f45c9da87a62664b017aa22e2a7843bf797f0e93
APITESTS_BRANCH=master
APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git
APITESTS_COMMITID=9151dd9c1dfa3cdc7e8e6e90d60181052af1854a
APITESTS_BRANCH=fix-test-expectations
APITESTS_REPO_GIT_URL=https://github.com/aduffeck/ocis.git
5 changes: 5 additions & 0 deletions changelog/unreleased/prevent-sharing-space-roots.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Prevent sharing space roots and personal spaces

We fixed a problem where sharing space roots or adding members to a personal space was possible.

https://github.com/cs3org/reva/pull/3849
7 changes: 7 additions & 0 deletions internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) {
return
}

// check that this is a valid share
if statRes.Info.Id.OpaqueId == statRes.Info.Id.SpaceId &&
(shareType != int(conversions.ShareTypeSpaceMembershipUser) && shareType != int(conversions.ShareTypeSpaceMembershipGroup)) {
response.WriteOCSError(w, r, http.StatusBadRequest, "Can not share space root", nil)
return
}

// check user has share permissions
if !conversions.RoleFromResourcePermissions(statRes.Info.PermissionSet, false).OCSPermissions().Contain(conversions.PermissionShare) {
response.WriteOCSError(w, r, http.StatusNotFound, "No share permission", nil)
Expand Down
318 changes: 206 additions & 112 deletions internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,57 +83,7 @@ var _ = Describe("The ocs API", func() {
})

Describe("CreateShare", func() {
var (
resID = &provider.ResourceId{
StorageId: "share1-storageid",
OpaqueId: "share1",
}
share = &collaboration.Share{
Id: &collaboration.ShareId{OpaqueId: "1"},
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
},
ResourceId: resID,
Permissions: &collaboration.SharePermissions{
Permissions: &provider.ResourcePermissions{
Stat: true,
ListContainer: true,
},
},
}
share2 = &collaboration.Share{
Id: &collaboration.ShareId{OpaqueId: "2"},
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
},
ResourceId: resID,
Permissions: &collaboration.SharePermissions{
Permissions: &provider.ResourcePermissions{
Stat: true,
ListContainer: true,
},
},
}
)

BeforeEach(func() {
client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{
Status: status.NewOK(context.Background()),
Info: &provider.ResourceInfo{
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
Path: "/newshare",
Id: resID,
Owner: user.Id,
PermissionSet: &provider.ResourcePermissions{
Stat: true,
AddGrant: true,
UpdateGrant: true,
RemoveGrant: true,
},
Size: 10,
},
}, nil)

client.On("GetUserByClaim", mock.Anything, mock.Anything).Return(&userpb.GetUserByClaimResponse{
Status: status.NewOK(context.Background()),
User: user,
Expand All @@ -146,104 +96,248 @@ var _ = Describe("The ocs API", func() {
Status: status.NewOK(context.Background()),
}, nil)

client.On("GetShare", mock.Anything, mock.Anything).Return(&collaboration.GetShareResponse{
Status: status.NewOK(context.Background()),
Share: share,
}, nil)

client.On("ListShares", mock.Anything, mock.Anything).Return(&collaboration.ListSharesResponse{
Status: status.NewOK(context.Background()),
}, nil)
})

Context("when there are no existing shares to the resource yet", func() {
Context("when sharing the space root", func() {
BeforeEach(func() {
client.On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything).Return(&collaboration.ListReceivedSharesResponse{
client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaboration.ReceivedShare{
{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share,
MountPoint: &provider.Reference{Path: ""},
Info: &provider.ResourceInfo{
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
Path: "/",
Id: &provider.ResourceId{
StorageId: "storageid",
SpaceId: "spaceid",
OpaqueId: "spaceid",
},
Owner: user.Id,
PermissionSet: &provider.ResourcePermissions{
Stat: true,
AddGrant: true,
UpdateGrant: true,
RemoveGrant: true,
},
Size: 10,
},
}, nil)
})

It("creates a new share", func() {
client.On("CreateShare", mock.Anything, mock.Anything).Return(&collaboration.CreateShareResponse{
Status: status.NewOK(context.Background()),
Share: share,
}, nil)

It("does not create a user share", func() {
form := url.Values{}
form.Add("shareType", "0")
form.Add("path", "/newshare")
form.Add("name", "newshare")
form.Add("permissions", "16")
form.Add("path", "/")
form.Add("spaceRef", "storageid!spaceid:spaceid")
form.Add("permissions", "1")
form.Add("role", "viewer")
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")
req = req.WithContext(ctx)

w := httptest.NewRecorder()
h.CreateShare(w, req)
Expect(w.Result().StatusCode).To(Equal(200))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 1)
Expect(w.Result().StatusCode).To(Equal(400))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 0)
})
})

Context("when a share to the same resource already exists", func() {
BeforeEach(func() {
client.On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything).Return(&collaboration.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaboration.ReceivedShare{
{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share,
MountPoint: &provider.Reference{Path: "some-mountpoint"},
Context("when sharing a resource", func() {
var (
resID = &provider.ResourceId{
StorageId: "share1-storageid",
OpaqueId: "share1",
}
share = &collaboration.Share{
Id: &collaboration.ShareId{OpaqueId: "1"},
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
},
ResourceId: resID,
Permissions: &collaboration.SharePermissions{
Permissions: &provider.ResourcePermissions{
Stat: true,
ListContainer: true,
},
{
State: collaboration.ShareState_SHARE_STATE_PENDING,
Share: share2,
},
}
share2 = &collaboration.Share{
Id: &collaboration.ShareId{OpaqueId: "2"},
Grantee: &provider.Grantee{
Type: provider.GranteeType_GRANTEE_TYPE_USER,
},
ResourceId: resID,
Permissions: &collaboration.SharePermissions{
Permissions: &provider.ResourcePermissions{
Stat: true,
ListContainer: true,
},
},
}, nil)
})
}
)

It("auto-accepts the share and applies the mountpoint", func() {
client.On("CreateShare", mock.Anything, mock.Anything).Return(&collaboration.CreateShareResponse{
BeforeEach(func() {
client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{
Status: status.NewOK(context.Background()),
Share: share2,
Info: &provider.ResourceInfo{
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
Path: "/newshare",
Id: resID,
Owner: user.Id,
PermissionSet: &provider.ResourcePermissions{
Stat: true,
AddGrant: true,
UpdateGrant: true,
RemoveGrant: true,
},
Size: 10,
},
}, nil)
client.On("UpdateReceivedShare", mock.Anything, mock.MatchedBy(func(req *collaboration.UpdateReceivedShareRequest) bool {
return req.Share.Share.Id.OpaqueId == "2" && req.Share.MountPoint.Path == "some-mountpoint" && req.Share.State == collaboration.ShareState_SHARE_STATE_ACCEPTED
})).Return(&collaboration.UpdateReceivedShareResponse{

client.On("GetShare", mock.Anything, mock.Anything).Return(&collaboration.GetShareResponse{
Status: status.NewOK(context.Background()),
Share: &collaboration.ReceivedShare{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share2,
MountPoint: &provider.Reference{Path: "share2"},
},
Share: share,
}, nil)
})

form := url.Values{}
form.Add("shareType", "0")
form.Add("path", "/newshare")
form.Add("name", "newshare")
form.Add("permissions", "16")
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")
req = req.WithContext(ctx)
Context("when there are no existing shares to the resource yet", func() {
BeforeEach(func() {
client.On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything).Return(&collaboration.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaboration.ReceivedShare{
{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share,
MountPoint: &provider.Reference{Path: ""},
},
},
}, nil)
})

It("creates a new share", func() {
client.On("CreateShare", mock.Anything, mock.Anything).Return(&collaboration.CreateShareResponse{
Status: status.NewOK(context.Background()),
Share: share,
}, nil)

form := url.Values{}
form.Add("shareType", "0")
form.Add("path", "/newshare")
form.Add("name", "newshare")
form.Add("permissions", "16")
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")
req = req.WithContext(ctx)

w := httptest.NewRecorder()
h.CreateShare(w, req)
Expect(w.Result().StatusCode).To(Equal(200))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 1)
})
})

w := httptest.NewRecorder()
h.CreateShare(w, req)
Expect(w.Result().StatusCode).To(Equal(200))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 1)
client.AssertNumberOfCalls(GinkgoT(), "UpdateReceivedShare", 1)
Context("when a share to the same resource already exists", func() {
BeforeEach(func() {
client.On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything).Return(&collaboration.ListReceivedSharesResponse{
Status: status.NewOK(context.Background()),
Shares: []*collaboration.ReceivedShare{
{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share,
MountPoint: &provider.Reference{Path: "some-mountpoint"},
},
{
State: collaboration.ShareState_SHARE_STATE_PENDING,
Share: share2,
},
},
}, nil)
})

It("auto-accepts the share and applies the mountpoint", func() {
client.On("CreateShare", mock.Anything, mock.Anything).Return(&collaboration.CreateShareResponse{
Status: status.NewOK(context.Background()),
Share: share2,
}, nil)
client.On("UpdateReceivedShare", mock.Anything, mock.MatchedBy(func(req *collaboration.UpdateReceivedShareRequest) bool {
return req.Share.Share.Id.OpaqueId == "2" && req.Share.MountPoint.Path == "some-mountpoint" && req.Share.State == collaboration.ShareState_SHARE_STATE_ACCEPTED
})).Return(&collaboration.UpdateReceivedShareResponse{
Status: status.NewOK(context.Background()),
Share: &collaboration.ReceivedShare{
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
Share: share2,
MountPoint: &provider.Reference{Path: "share2"},
},
}, nil)

form := url.Values{}
form.Add("shareType", "0")
form.Add("path", "/newshare")
form.Add("name", "newshare")
form.Add("permissions", "16")
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")
req = req.WithContext(ctx)

w := httptest.NewRecorder()
h.CreateShare(w, req)
Expect(w.Result().StatusCode).To(Equal(200))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 1)
client.AssertNumberOfCalls(GinkgoT(), "UpdateReceivedShare", 1)
})
})
})

It("does not allow adding space members to a personal space", func() {
client.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{
Status: status.NewOK(context.Background()),
Info: &provider.ResourceInfo{
Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER,
Path: "/",
Id: &provider.ResourceId{
StorageId: "storageid",
SpaceId: "spaceid",
OpaqueId: "spaceid",
},
Owner: user.Id,
PermissionSet: &provider.ResourcePermissions{
Stat: true,
GetPath: true,
GetQuota: true,
InitiateFileDownload: true,
ListRecycle: true,
ListContainer: true,
AddGrant: true,
UpdateGrant: true,
RemoveGrant: true,
},
Size: 10,
Space: &provider.StorageSpace{
SpaceType: "personal",
},
},
}, nil)

form := url.Values{}
form.Add("shareType", "7")
form.Add("path", "/")
form.Add("spaceRef", "storageid!spaceid")
form.Add("permissions", "1")
form.Add("role", "viewer")
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")
req = req.WithContext(ctx)

w := httptest.NewRecorder()
h.CreateShare(w, req)
Expect(w.Result().StatusCode).To(Equal(400))
client.AssertNumberOfCalls(GinkgoT(), "CreateShare", 0)
})
})

Describe("ListShares", func() {
Expand Down
Loading

0 comments on commit 98d8707

Please sign in to comment.