Compare commits

...
Author SHA1 Message Date
Dominik Schmidt b8eb8f8261 test(graph): root children permission denied is 404 2026-09-07 16:50:44 +02:00
Dominik Schmidt d885cf0256 fix(graph): expose allowedValues on driveItem children
GetRootDriveChildren and GetDriveItem fill
@libre.graph.permissions.actions.allowedValues on $select, the shared
children listing did not, so every child in /children and $expand=children
came back without its allowed actions and clients treated the listing as read
only. Add a setDriveItem(s)AllowedValues helper pair, matching the thumbnails
and shareTypes shape, and apply it in the listing.

GetRootDriveChildren now delegates to the shared children listing instead of
repeating its ListContainer + format + shareTypes tail; permission denied on
the root listing answers 404 like the item children endpoint (the TODO that
sat there), not 403.
2026-09-07 16:50:44 +02:00
2 changed files with 55 additions and 46 deletions

No files matched your search

+24 -44
View File
@@ -222,50 +222,9 @@ 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())
files, ok := g.listDriveItemChildren(w, r, space.GetRoot())
if !ok {
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())
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)
@@ -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 {
@@ -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),