From 32b2fcda4004963a247a9345e2c8026878697f92 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 8 Feb 2024 11:00:27 +0100 Subject: [PATCH] enhancement(sharing): Simplify route for accepting shares In theory creating the driveItem to accepting a shared resource can be done via '/v1beta1/drives/{drive-id}/item/{item-id}/children' but there is also the simplified variant via '/v1beta1/drives/{drive-id}/root/children' (aligned with the example in the spec). For now we'll just implement the latter because accepting a share will always be done via root of the sharejail drive. --- .../pkg/service/v0/api_drives_drive_item.go | 13 +++++----- .../service/v0/api_drives_drive_item_test.go | 9 ++----- services/graph/pkg/service/v0/service.go | 24 ++++++++++--------- 3 files changed, 21 insertions(+), 25 deletions(-) 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 ac83f58f872..beab116bfd8 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item.go @@ -207,17 +207,16 @@ func (api DrivesDriveItemApi) DeleteDriveItem(w http.ResponseWriter, r *http.Req // CreateDriveItem creates a drive item func (api DrivesDriveItemApi) CreateDriveItem(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - driveID, itemID, err := GetDriveAndItemIDParam(r, &api.logger) + driveID, err := parseIDParam(r, "driveID") if err != nil { - msg := "invalid driveID or itemID" - api.logger.Debug().Err(err).Msg(msg) - errorcode.InvalidRequest.Render(w, r, http.StatusUnprocessableEntity, msg) + api.logger.Debug().Err(err).Msg("invlid driveID") + errorcode.InvalidRequest.Render(w, r, http.StatusUnprocessableEntity, "invalid driveID") return } - if !IsShareJail(driveID) || !IsShareJail(itemID) { - msg := "invalid driveID or itemID, must be share jail" - api.logger.Debug().Interface("driveID", driveID).Interface("itemID", itemID).Msg(msg) + if !IsShareJail(driveID) { + msg := "invalid driveID, must be share jail" + api.logger.Debug().Interface("driveID", driveID).Msg(msg) errorcode.InvalidRequest.Render(w, r, http.StatusUnprocessableEntity, 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 e06d3936230..24c90e7f810 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 @@ -346,7 +346,6 @@ var _ = Describe("DrivesDriveItemApi", func() { rCTX = chi.NewRouteContext() rCTX.URLParams.Add("driveID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668") - rCTX.URLParams.Add("itemID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!a0ca6a90-a365-4782-871e-d44447bbc668") }) checkDriveIDAndItemIDValidation := func(handler http.HandlerFunc) { @@ -367,12 +366,13 @@ var _ = Describe("DrivesDriveItemApi", func() { Expect(jsonData.Get("message").String()).To(Equal("invalid driveID or itemID")) } - Describe("CreateDriveItem", func() { + Describe("DeleteDriveItem", func() { It("validates the driveID and itemID url param", func() { checkDriveIDAndItemIDValidation(httpAPI.DeleteDriveItem) }) It("uses the UnmountShare provider implementation", func() { + rCTX.URLParams.Add("itemID", "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!a0ca6a90-a365-4782-871e-d44447bbc668") responseRecorder := httptest.NewRecorder() request := httptest.NewRequest(http.MethodDelete, "/", nil). @@ -409,13 +409,8 @@ var _ = Describe("DrivesDriveItemApi", func() { }) Describe("CreateDriveItem", func() { - It("validates the driveID and itemID url param", func() { - checkDriveIDAndItemIDValidation(httpAPI.CreateDriveItem) - }) - It("checks if the idemID and driveID is in share jail", func() { rCTX.URLParams.Add("driveID", "1$2") - rCTX.URLParams.Add("itemID", "1$2!3") responseRecorder := httptest.NewRecorder() request := httptest.NewRequest(http.MethodPost, "/", nil). diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index cdca38915c5..b1abfc73662 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -227,19 +227,21 @@ func NewService(opts ...Option) (Graph, error) { }) r.Route("/drives", func(r chi.Router) { r.Get("/", svc.GetAllDrives(APIVersion_1_Beta_1)) - r.Route("/{driveID}/items/{itemID}", func(r chi.Router) { - r.Delete("/", drivesDriveItemApi.DeleteDriveItem) - r.Post("/children", drivesDriveItemApi.CreateDriveItem) - r.Post("/invite", svc.Invite) - r.Route("/permissions", func(r chi.Router) { - r.Get("/", svc.ListPermissions) - r.Route("/{permissionID}", func(r chi.Router) { - r.Delete("/", svc.DeletePermission) - r.Patch("/", svc.UpdatePermission) - r.Post("/setPassword", svc.SetLinkPassword) + r.Route("/{driveID}", func(r chi.Router) { + r.Post("/root/children", drivesDriveItemApi.CreateDriveItem) + r.Route("/items/{itemID}", func(r chi.Router) { + r.Delete("/", drivesDriveItemApi.DeleteDriveItem) + r.Post("/invite", svc.Invite) + r.Route("/permissions", func(r chi.Router) { + r.Get("/", svc.ListPermissions) + r.Route("/{permissionID}", func(r chi.Router) { + r.Delete("/", svc.DeletePermission) + r.Patch("/", svc.UpdatePermission) + r.Post("/setPassword", svc.SetLinkPassword) + }) }) + r.Post("/createLink", svc.CreateLink) }) - r.Post("/createLink", svc.CreateLink) }) }) r.Route("/roleManagement/permissions/roleDefinitions", func(r chi.Router) {