Prevent sharing space roots

.drone.env

@@ -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

changelog/unreleased/prevent-sharing-space-roots.md

@@ -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

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -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)

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go

@@ -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,
@@ -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() {