diff --git a/changelog/unreleased/fix-graph-create-drive-item.md b/changelog/unreleased/fix-graph-create-drive-item.md new file mode 100644 index 00000000000..e6b792603c6 --- /dev/null +++ b/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 diff --git a/services/graph/pkg/service/v0/api_drives_drive_item.go b/services/graph/pkg/service/v0/api_drives_drive_item.go index 76c2516ffb2..c7c28f43a4e 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item.go @@ -125,7 +125,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 { @@ -158,6 +160,9 @@ 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 @@ -165,6 +170,7 @@ func (s DrivesDriveItemService) MountShare(ctx context.Context, resourceID stora // 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 @@ -284,7 +290,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 } @@ -293,7 +299,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 } @@ -302,7 +308,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 } diff --git a/services/graph/pkg/service/v0/api_drives_drive_item_test.go b/services/graph/pkg/service/v0/api_drives_drive_item_test.go index c526d49c135..960d58d7259 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item_test.go +++ b/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"))