diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index b2c21017cb..da12b2c7b9 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -35,11 +35,14 @@ import ( // opt-in driveItem instance annotations, returned only when requested via $select const _selectAllowedValues = "@libre.graph.permissions.actions.allowedValues" -// driveItemPropertySelected reports whether the given opt-in property was requested via $select -func driveItemPropertySelected(r *http.Request, property string) bool { - for _, values := range r.URL.Query()["$select"] { +// opt-in driveItem relations, returned only when requested via $expand +const _expandChildren = "children" + +// odataListContains reports whether the given comma separated odata query parameter contains value +func odataListContains(r *http.Request, parameter, value string) bool { + for _, values := range r.URL.Query()[parameter] { for _, v := range strings.Split(values, ",") { - if v == property { + if v == value { return true } } @@ -47,6 +50,16 @@ func driveItemPropertySelected(r *http.Request, property string) bool { return false } +// driveItemPropertySelected reports whether the given opt-in property was requested via $select +func driveItemPropertySelected(r *http.Request, property string) bool { + return odataListContains(r, "$select", property) +} + +// driveItemRelationExpanded reports whether the given opt-in relation was requested via $expand +func driveItemRelationExpanded(r *http.Request, relation string) bool { + return odataListContains(r, "$expand", relation) +} + // CreateUploadSession create an upload session to allow your app to upload files up to the maximum file size. // An upload session allows your app to upload ranges of the file in sequential API requests, which allows the // transfer to be resumed if a connection is dropped while the upload is in progress. @@ -302,6 +315,18 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { driveItem.LibreGraphPermissionsActionsAllowedValues = unifiedrole.CS3ResourcePermissionsToLibregraphActions(res.GetInfo().GetPermissionSet()) } + // only containers have children, for anything else the relation stays unset + if driveItemRelationExpanded(r, _expandChildren) && res.GetInfo().GetType() == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER { + children, ok := g.listDriveItemChildren(w, r, &driveItemID) + if !ok { + return + } + driveItem.Children = make([]libregraph.DriveItem, 0, len(children)) + for _, child := range children { + driveItem.Children = append(driveItem.Children, *child) + } + } + render.Status(r, http.StatusOK) render.JSON(w, r, &driveItem) } @@ -309,7 +334,6 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { // GetDriveItemChildren lists the children of a driveItem func (g Graph) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) { g.logger.Info().Msg("Calling GetDriveItemChildren") - ctx := r.Context() driveID, err := parseIDParam(r, "driveID") if err != nil { @@ -335,38 +359,8 @@ func (g Graph) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) { } */ - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) - return - } - - res, err := gatewayClient.ListContainer(ctx, &storageprovider.ListContainerRequest{ - Ref: &storageprovider.Reference{ResourceId: &driveItemID}, - }) - switch { - case err != nil: - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) - return - case res.GetStatus().GetCode() == cs3rpc.Code_CODE_OK: - // ok - case res.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND: - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) - return - case res.GetStatus().GetCode() == cs3rpc.Code_CODE_PERMISSION_DENIED: - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) // do not leak existence? check what graph does - return - case res.GetStatus().GetCode() == cs3rpc.Code_CODE_UNAUTHENTICATED: - errorcode.Unauthenticated.Render(w, r, http.StatusUnauthorized, res.GetStatus().GetMessage()) // do not leak existence? check what graph does - return - default: - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.GetStatus().GetMessage()) - return - } - - files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos()) - if err != nil { - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + files, ok := g.listDriveItemChildren(w, r, &driveItemID) + if !ok { return } @@ -374,6 +368,48 @@ func (g Graph) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, &ListResponse{Value: files}) } +// listDriveItemChildren lists the children of the given container. Shared by +// the children endpoint and by $expand=children so both return the same items. +// It renders the error response itself and reports whether it succeeded. +func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, driveItemID *storageprovider.ResourceId) ([]*libregraph.DriveItem, bool) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return nil, false + } + + res, err := gatewayClient.ListContainer(r.Context(), &storageprovider.ListContainerRequest{ + Ref: &storageprovider.Reference{ResourceId: driveItemID}, + }) + switch { + case err != nil: + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return nil, false + case res.GetStatus().GetCode() == cs3rpc.Code_CODE_OK: + // ok + case res.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND: + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) + return nil, false + case res.GetStatus().GetCode() == cs3rpc.Code_CODE_PERMISSION_DENIED: + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) // do not leak existence? check what graph does + return nil, false + case res.GetStatus().GetCode() == cs3rpc.Code_CODE_UNAUTHENTICATED: + errorcode.Unauthenticated.Render(w, r, http.StatusUnauthorized, res.GetStatus().GetMessage()) // do not leak existence? check what graph does + return nil, false + default: + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.GetStatus().GetMessage()) + return nil, false + } + + files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos()) + if err != nil { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return nil, false + } + + return files, true +} + func (g Graph) getRemoteItem(ctx context.Context, root *storageprovider.ResourceId, baseURL *url.URL) (*libregraph.RemoteItem, error) { gatewayClient, err := g.gatewaySelector.Next() if err != nil { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index 574237dd59..369b35e9ee 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -243,6 +243,100 @@ var _ = Describe("Driveitems", func() { }) }) + Describe("GetDriveItem", func() { + var ( + folderInfo *provider.ResourceInfo + mtime = time.Now() + ) + + newRequest := func(query string) *http.Request { + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/drives/storageid$spaceid/items/storageid$spaceid!nodeid"+query, nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "storageid$spaceid") + rctx.URLParams.Add("driveItemID", "storageid$spaceid!nodeid") + return r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + } + + getItem := func(r *http.Request) libregraph.DriveItem { + svc.GetDriveItem(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + data, err := io.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + item := libregraph.DriveItem{} + Expect(json.Unmarshal(data, &item)).To(Succeed()) + return item + } + + BeforeEach(func() { + folderInfo = &provider.ResourceInfo{ + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + Id: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "nodeid"}, + Etag: "etag", + Mtime: utils.TimeToTS(mtime), + } + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: folderInfo, + }, nil) + gatewayClient.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{ + Status: status.NewOK(ctx), + Infos: []*provider.ResourceInfo{ + { + Type: provider.ResourceType_RESOURCE_TYPE_FILE, + Id: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"}, + Etag: "etag", + Mtime: utils.TimeToTS(mtime), + }, + }, + }, nil) + }) + + It("leaves children unset without $expand", func() { + Expect(getItem(newRequest("")).Children).To(BeNil()) + gatewayClient.AssertNotCalled(GinkgoT(), "ListContainer", mock.Anything, mock.Anything) + }) + + It("returns the children when requested via $expand", func() { + item := getItem(newRequest("?$expand=children")) + Expect(item.Children).To(HaveLen(1)) + Expect(item.Children[0].GetId()).To(Equal("storageid$spaceid!opaqueid")) + Expect(item.Children[0].GetETag()).To(Equal("etag")) + }) + + It("leaves children unset for a file", func() { + folderInfo.Type = provider.ResourceType_RESOURCE_TYPE_FILE + + Expect(getItem(newRequest("?$expand=children")).Children).To(BeNil()) + gatewayClient.AssertNotCalled(GinkgoT(), "ListContainer", mock.Anything, mock.Anything) + }) + }) + + Describe("GetDriveItem $expand=children error", func() { + It("propagates a failing child listing", func() { + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: &provider.ResourceInfo{ + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + Id: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "nodeid"}, + Mtime: utils.TimeToTS(time.Now()), + }, + }, nil) + gatewayClient.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/drives/storageid$spaceid/items/storageid$spaceid!nodeid?$expand=children", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "storageid$spaceid") + rctx.URLParams.Add("driveItemID", "storageid$spaceid!nodeid") + r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx)) + + svc.GetDriveItem(rr, r) + Expect(rr.Code).To(Equal(http.StatusNotFound)) + }) + }) + Describe("GetDriveItemChildren", func() { It("handles ListContainer not found", func() { gatewayClient.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{