Merge pull request #8817 from 2403905/issue-8724

fix creating the drive item

changelog/unreleased/fix-graph-create-drive-item.md

@@ -0,0 +1,6 @@
+Bugfix: Fix creating the drive item
+
+We fixed the issue when creating a drive item with random item id was allowed
+
+https://github.com/owncloud/ocis/pull/8817
+https://github.com/owncloud/ocis/issues/8724

services/graph/pkg/service/v0/api_drives_drive_item.go

@@ -129,7 +129,9 @@ func (s DrivesDriveItemService) MountShare(ctx context.Context, resourceID stora
 	if filepath.IsAbs(name) {
 		return libregraph.DriveItem{}, errorcode.New(errorcode.InvalidRequest, "name cannot be an absolute path")
 	}
-	name = filepath.Clean(name)
+	if name != "" {
+		name = filepath.Clean(name)
+	}
 
 	gatewayClient, err := s.gatewaySelector.Next()
 	if err != nil {
@@ -162,13 +164,17 @@ func (s DrivesDriveItemService) MountShare(ctx context.Context, resourceID stora
 	if err != nil {
 		return libregraph.DriveItem{}, err
 	}
+	if len(receivedSharesResponse.GetShares()) == 0 {
+		return libregraph.DriveItem{}, errorcode.New(errorcode.InvalidRequest, "invalid itemID")
+	}
 
 	var errs []error
 
 	var acceptedShares []*collaboration.ReceivedShare
 
 	// try to accept all the received shares for this resource. So that the stat is in sync across all
 	// shares
+
 	for _, receivedShare := range receivedSharesResponse.GetShares() {
 		updateMask := &fieldmaskpb.FieldMask{Paths: []string{_fieldMaskPathState}}
 		receivedShare.State = collaboration.ShareState_SHARE_STATE_ACCEPTED
@@ -292,7 +298,7 @@ func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Req
 	if err := StrictJSONUnmarshal(r.Body, &requestDriveItem); err != nil {
 		msg := "invalid request body"
 		api.logger.Debug().Err(err).Msg(msg)
-		errorcode.InvalidRequest.Render(w, r, http.StatusUnprocessableEntity, msg)
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, msg)
 		return
 	}
 
@@ -301,7 +307,7 @@ func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Req
 	if err != nil {
 		msg := "invalid remote item id"
 		api.logger.Debug().Err(err).Msg(msg)
-		errorcode.InvalidRequest.Render(w, r, http.StatusUnprocessableEntity, msg)
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, msg)
 		return
 	}
 
@@ -310,7 +316,7 @@ func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Req
 	if err != nil {
 		msg := "mounting share failed"
 		api.logger.Debug().Err(err).Msg(msg)
-		errorcode.InvalidRequest.Render(w, r, http.StatusFailedDependency, msg)
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, msg)
 		return
 	}
 

services/graph/pkg/service/v0/api_drives_drive_item_test.go

@@ -80,6 +80,9 @@ var _ = Describe("DrivesDriveItemService", func() {
 					OpaqueId:  "2",
 					SpaceId:   "3",
 				}
+				expectedShareID := collaborationv1beta1.ShareId{
+					OpaqueId: "1:2:3",
+				}
 				gatewayClient.
 					On("ListReceivedShares", mock.Anything, mock.Anything, mock.Anything).
 					Return(func(ctx context.Context, in *collaborationv1beta1.ListReceivedSharesRequest, opts ...grpc.CallOption) (*collaborationv1beta1.ListReceivedSharesResponse, error) {
@@ -106,7 +109,44 @@ var _ = Describe("DrivesDriveItemService", func() {
 						Expect(resourceIDs).To(HaveLen(1))
 						Expect(resourceIDs[0]).To(Equal(&expectedResourceID))
 
-						return nil, nil
+						return &collaborationv1beta1.ListReceivedSharesResponse{
+							Shares: []*collaborationv1beta1.ReceivedShare{
+								{
+									State: collaborationv1beta1.ShareState_SHARE_STATE_PENDING,
+									Share: &collaborationv1beta1.Share{
+										Id: &expectedShareID,
+									},
+								},
+							},
+						}, nil
+					})
+				gatewayClient.
+					On("UpdateReceivedShare", mock.Anything, mock.Anything, mock.Anything).
+					Return(func(ctx context.Context, in *collaborationv1beta1.UpdateReceivedShareRequest, opts ...grpc.CallOption) (*collaborationv1beta1.UpdateReceivedShareResponse, error) {
+						Expect(in.GetUpdateMask().GetPaths()).To(Equal([]string{"state"}))
+						Expect(in.GetShare().GetState()).To(Equal(collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED))
+						Expect(in.GetShare().GetShare().GetId().GetOpaqueId()).To(Equal(expectedShareID.GetOpaqueId()))
+						return &collaborationv1beta1.UpdateReceivedShareResponse{
+							Status: status.NewOK(ctx),
+							Share: &collaborationv1beta1.ReceivedShare{
+								State: collaborationv1beta1.ShareState_SHARE_STATE_ACCEPTED,
+								Share: &collaborationv1beta1.Share{
+									Id:         &expectedShareID,
+									ResourceId: &expectedResourceID,
+								},
+							},
+						}, nil
+					})
+				gatewayClient.
+					On("Stat", mock.Anything, mock.Anything, mock.Anything).
+					Return(func(ctx context.Context, in *storageprovider.StatRequest, opts ...grpc.CallOption) (*storageprovider.StatResponse, error) {
+						return &storageprovider.StatResponse{
+							Status: status.NewOK(ctx),
+							Info: &storageprovider.ResourceInfo{
+								Id:   &expectedResourceID,
+								Name: "name",
+							},
+						}, nil
 					})
 
 				_, err := drivesDriveItemService.MountShare(context.Background(), expectedResourceID, "")
@@ -634,7 +674,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
 
 			httpAPI.CreateDriveItem(responseRecorder, request)
 
-			Expect(responseRecorder.Code).To(Equal(http.StatusUnprocessableEntity))
+			Expect(responseRecorder.Code).To(Equal(http.StatusBadRequest))
 
 			jsonData := gjson.Get(responseRecorder.Body.String(), "error")
 			Expect(jsonData.Get("message").String()).To(Equal("invalid request body"))
@@ -654,7 +694,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
 
 			httpAPI.CreateDriveItem(responseRecorder, request)
 
-			Expect(responseRecorder.Code).To(Equal(http.StatusUnprocessableEntity))
+			Expect(responseRecorder.Code).To(Equal(http.StatusBadRequest))
 
 			jsonData = gjson.Get(responseRecorder.Body.String(), "error")
 			Expect(jsonData.Get("message").String()).To(Equal("invalid remote item id"))
@@ -688,7 +728,7 @@ var _ = Describe("DrivesDriveItemApi", func() {
 
 			httpAPI.CreateDriveItem(responseRecorder, request)
 
-			Expect(responseRecorder.Code).To(Equal(http.StatusFailedDependency))
+			Expect(responseRecorder.Code).To(Equal(http.StatusBadRequest))
 
 			jsonData := gjson.Get(responseRecorder.Body.String(), "error")
 			Expect(jsonData.Get("message").String()).To(Equal("mounting share failed"))