diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 167f8b1b62..02f535081d 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -222,51 +222,10 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { } } - listRequest := &storageprovider.ListContainerRequest{ - Ref: &storageprovider.Reference{ResourceId: space.GetRoot()}, - } - if driveItemPropertySelected(r, _selectShareTypes) { - listRequest.FieldMask = shareTypesFieldMask - } - - lRes, err := gatewayClient.ListContainer(ctx, listRequest) - switch { - case err != nil: - g.logger.Error().Err(err).Msg("error making ListContainer grpc call") - errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) - return - case lRes.GetStatus().GetCode() != cs3rpc.Code_CODE_OK: - if lRes.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND { - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, lRes.GetStatus().GetMessage()) - return - } - if lRes.GetStatus().GetCode() == cs3rpc.Code_CODE_PERMISSION_DENIED { - // TODO check if we should return 404 to not disclose existing items - errorcode.AccessDenied.Render(w, r, http.StatusForbidden, lRes.GetStatus().GetMessage()) - return - } - g.logger.Error().Err(err).Msg("error sending list container grpc request") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.GetStatus().GetMessage()) - return - } - - files, err := formatDriveItems(g.logger, g.publicBaseURL, lRes.GetInfos()) - if err != nil { - g.logger.Error().Err(err).Msg("error encoding response as json") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + files, ok := g.listDriveItemChildren(w, r, space.GetRoot()) + if !ok { return } - g.setDriveItemsThumbnails(r, files, lRes.GetInfos()) - - if driveItemPropertySelected(r, _selectAllowedValues) { - for i, info := range lRes.GetInfos() { - files[i].LibreGraphPermissionsActionsAllowedValues = unifiedrole.CS3ResourcePermissionsToLibregraphActions(info.GetPermissionSet()) - } - } - - if driveItemPropertySelected(r, _selectShareTypes) { - g.addShareTypes(ctx, files, lRes.GetInfos()) - } render.Status(r, http.StatusOK) render.JSON(w, r, &ListResponse{Value: files}) @@ -333,7 +292,7 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { } if driveItemPropertySelected(r, _selectAllowedValues) { - driveItem.LibreGraphPermissionsActionsAllowedValues = unifiedrole.CS3ResourcePermissionsToLibregraphActions(res.GetInfo().GetPermissionSet()) + setDriveItemAllowedValues(driveItem, res.GetInfo()) } // only containers have children @@ -436,6 +395,8 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri return nil, false } + setDriveItemsAllowedValues(r, files, res.GetInfos()) + if driveItemPropertySelected(r, _selectShareTypes) { g.addShareTypes(r.Context(), files, res.GetInfos()) } @@ -445,6 +406,25 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri return files, true } +// setDriveItemAllowedValues fills @libre.graph.permissions.actions.allowedValues +// from the item's permission set. +func setDriveItemAllowedValues(item *libregraph.DriveItem, info *storageprovider.ResourceInfo) { + item.LibreGraphPermissionsActionsAllowedValues = unifiedrole.CS3ResourcePermissionsToLibregraphActions(info.GetPermissionSet()) +} + +// setDriveItemsAllowedValues does the same across a listing, when the property +// was selected. +func setDriveItemsAllowedValues(r *http.Request, items []libregraph.DriveItem, infos []*storageprovider.ResourceInfo) { + if !driveItemPropertySelected(r, _selectAllowedValues) { + return + } + for i := range items { + if i < len(infos) { + setDriveItemAllowedValues(&items[i], infos[i]) + } + } +} + 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 42e698a096..94a56999e7 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -142,7 +142,7 @@ var _ = Describe("Driveitems", func() { Expect(rr.Code).To(Equal(http.StatusNotFound)) }) - It("handles ListContainer permission denied", func() { + It("handles ListContainer permission denied as not found", func() { gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ Status: status.NewOK(ctx), StorageSpaces: []*provider.StorageSpace{{Owner: currentUser, Root: &provider.ResourceId{}}}, @@ -154,7 +154,7 @@ var _ = Describe("Driveitems", func() { r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drive/root/children", nil) r = r.WithContext(revactx.ContextSetUser(ctx, currentUser)) svc.GetRootDriveChildren(rr, r) - Expect(rr.Code).To(Equal(http.StatusForbidden)) + Expect(rr.Code).To(Equal(http.StatusNotFound)) }) It("handles ListContainer error", func() { @@ -515,6 +515,35 @@ var _ = Describe("Driveitems", func() { Expect(res.Value[0].PendingOperations).To(BeNil()) }) + It("returns the allowed actions when selected", func() { + r = r.WithContext(r.Context()) + q := r.URL.Query() + q.Add("$select", "@libre.graph.permissions.actions.allowedValues") + r.URL.RawQuery = q.Encode() + + 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), + PermissionSet: &provider.ResourcePermissions{ + GetPath: true, + InitiateFileDownload: true, + }, + }, + }, + }, nil) + + res := assertItemsList(1) + Expect(res.Value[0].GetLibreGraphPermissionsActionsAllowedValues()).To(ConsistOf( + unifiedrole.DriveItemPathRead, + unifiedrole.DriveItemContentRead, + )) + }) + It("omits share types unless they are selected", func() { gatewayClient.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{ Status: status.NewOK(ctx),