diff --git a/services/graph/mocks/drive_item_permissions_provider.go b/services/graph/mocks/drive_item_permissions_provider.go index afaf5ba5e..cd2429b01 100644 --- a/services/graph/mocks/drive_item_permissions_provider.go +++ b/services/graph/mocks/drive_item_permissions_provider.go @@ -9,6 +9,8 @@ import ( mock "github.com/stretchr/testify/mock" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + + svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) // DriveItemPermissionsProvider is an autogenerated mock type for the DriveItemPermissionsProvider type @@ -294,9 +296,9 @@ func (_c *DriveItemPermissionsProvider_Invite_Call) RunAndReturn(run func(contex return _c } -// ListPermissions provides a mock function with given fields: ctx, itemID, listFederatedRoles, selectedAttrs -func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { - ret := _m.Called(ctx, itemID, listFederatedRoles, selectedAttrs) +// ListPermissions provides a mock function with given fields: ctx, itemID, queryOptions +func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId, queryOptions svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + ret := _m.Called(ctx, itemID, queryOptions) if len(ret) == 0 { panic("no return value specified for ListPermissions") @@ -304,17 +306,17 @@ func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, ite var r0 libregraph.CollectionOfPermissionsWithAllowedValues var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { - return rf(ctx, itemID, listFederatedRoles, selectedAttrs) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { + return rf(ctx, itemID, queryOptions) } - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) libregraph.CollectionOfPermissionsWithAllowedValues); ok { - r0 = rf(ctx, itemID, listFederatedRoles, selectedAttrs) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) libregraph.CollectionOfPermissionsWithAllowedValues); ok { + r0 = rf(ctx, itemID, queryOptions) } else { r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues) } - if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, bool, []string) error); ok { - r1 = rf(ctx, itemID, listFederatedRoles, selectedAttrs) + if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) error); ok { + r1 = rf(ctx, itemID, queryOptions) } else { r1 = ret.Error(1) } @@ -330,15 +332,14 @@ type DriveItemPermissionsProvider_ListPermissions_Call struct { // ListPermissions is a helper method to define mock.On call // - ctx context.Context // - itemID *providerv1beta1.ResourceId -// - listFederatedRoles bool -// - selectedAttrs []string -func (_e *DriveItemPermissionsProvider_Expecter) ListPermissions(ctx interface{}, itemID interface{}, listFederatedRoles interface{}, selectedAttrs interface{}) *DriveItemPermissionsProvider_ListPermissions_Call { - return &DriveItemPermissionsProvider_ListPermissions_Call{Call: _e.mock.On("ListPermissions", ctx, itemID, listFederatedRoles, selectedAttrs)} +// - queryOptions svc.ListPermissionsQueryOptions +func (_e *DriveItemPermissionsProvider_Expecter) ListPermissions(ctx interface{}, itemID interface{}, queryOptions interface{}) *DriveItemPermissionsProvider_ListPermissions_Call { + return &DriveItemPermissionsProvider_ListPermissions_Call{Call: _e.mock.On("ListPermissions", ctx, itemID, queryOptions)} } -func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectedAttrs []string)) *DriveItemPermissionsProvider_ListPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId, queryOptions svc.ListPermissionsQueryOptions)) *DriveItemPermissionsProvider_ListPermissions_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(bool), args[3].([]string)) + run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(svc.ListPermissionsQueryOptions)) }) return _c } @@ -348,14 +349,14 @@ func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Return(_a0 libregra return _c } -func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, bool, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_Call { _c.Call.Return(run) return _c } -// ListSpaceRootPermissions provides a mock function with given fields: ctx, driveID, selectedAttrs -func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { - ret := _m.Called(ctx, driveID, selectedAttrs) +// ListSpaceRootPermissions provides a mock function with given fields: ctx, driveID, queryOptions +func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId, queryOptions svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + ret := _m.Called(ctx, driveID, queryOptions) if len(ret) == 0 { panic("no return value specified for ListSpaceRootPermissions") @@ -363,17 +364,17 @@ func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Con var r0 libregraph.CollectionOfPermissionsWithAllowedValues var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { - return rf(ctx, driveID, selectedAttrs) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { + return rf(ctx, driveID, queryOptions) } - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, []string) libregraph.CollectionOfPermissionsWithAllowedValues); ok { - r0 = rf(ctx, driveID, selectedAttrs) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) libregraph.CollectionOfPermissionsWithAllowedValues); ok { + r0 = rf(ctx, driveID, queryOptions) } else { r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues) } - if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, []string) error); ok { - r1 = rf(ctx, driveID, selectedAttrs) + if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) error); ok { + r1 = rf(ctx, driveID, queryOptions) } else { r1 = ret.Error(1) } @@ -389,14 +390,14 @@ type DriveItemPermissionsProvider_ListSpaceRootPermissions_Call struct { // ListSpaceRootPermissions is a helper method to define mock.On call // - ctx context.Context // - driveID *providerv1beta1.ResourceId -// - selectedAttrs []string -func (_e *DriveItemPermissionsProvider_Expecter) ListSpaceRootPermissions(ctx interface{}, driveID interface{}, selectedAttrs interface{}) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { - return &DriveItemPermissionsProvider_ListSpaceRootPermissions_Call{Call: _e.mock.On("ListSpaceRootPermissions", ctx, driveID, selectedAttrs)} +// - queryOptions svc.ListPermissionsQueryOptions +func (_e *DriveItemPermissionsProvider_Expecter) ListSpaceRootPermissions(ctx interface{}, driveID interface{}, queryOptions interface{}) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { + return &DriveItemPermissionsProvider_ListSpaceRootPermissions_Call{Call: _e.mock.On("ListSpaceRootPermissions", ctx, driveID, queryOptions)} } -func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId, selectedAttrs []string)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId, queryOptions svc.ListPermissionsQueryOptions)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].([]string)) + run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(svc.ListPermissionsQueryOptions)) }) return _c } @@ -406,7 +407,7 @@ func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Return(_a0 return _c } -func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { _c.Call.Return(run) return _c } diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 4ef7ba803..eac95fb3c 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -52,8 +52,8 @@ const ( type DriveItemPermissionsProvider interface { Invite(ctx context.Context, resourceId *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) SpaceRootInvite(ctx context.Context, driveID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) - ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) - ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) + ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, queryOptions ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) + ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, queryOptions ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) DeletePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) error DeleteSpaceRootPermission(ctx context.Context, driveID *storageprovider.ResourceId, permissionID string) error UpdatePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string, newPermission libregraph.Permission) (libregraph.Permission, error) @@ -79,6 +79,13 @@ const ( OCM ) +type ListPermissionsQueryOptions struct { + count bool + noValues bool + filterFederatedRoles bool + selectedAttrs []string +} + // NewDriveItemPermissionsService creates a new DriveItemPermissionsService func NewDriveItemPermissionsService(logger log.Logger, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], identityCache identity.IdentityCache, config *config.Config) (DriveItemPermissionsService, error) { return DriveItemPermissionsService{ @@ -345,7 +352,7 @@ func (s DriveItemPermissionsService) SpaceRootInvite(ctx context.Context, driveI } // ListPermissions lists the permissions of a driveItem -func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles bool, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { +func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, queryOptions ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{} gatewayClient, err := s.gatewaySelector.Next() if err != nil { @@ -368,17 +375,17 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID collectionOfPermissions = libregraph.CollectionOfPermissionsWithAllowedValues{} - if len(selectedAttrs) == 0 || slices.Contains(selectedAttrs, "@libre.graph.permissions.actions.allowedValues") { + if len(queryOptions.selectedAttrs) == 0 || slices.Contains(queryOptions.selectedAttrs, "@libre.graph.permissions.actions.allowedValues") { collectionOfPermissions.LibreGraphPermissionsActionsAllowedValues = allowedActions } - if len(selectedAttrs) == 0 || slices.Contains(selectedAttrs, "@libre.graph.permissions.roles.allowedValues") { + if len(queryOptions.selectedAttrs) == 0 || slices.Contains(queryOptions.selectedAttrs, "@libre.graph.permissions.roles.allowedValues") { collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues = conversions.ToValueSlice( unifiedrole.GetRolesByPermissions( unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(s.config.UnifiedRoles.AvailableRoles...)), allowedActions, condition, - listFederatedRoles, + queryOptions.filterFederatedRoles, false, ), ) @@ -390,7 +397,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues[i] = definition } - if len(selectedAttrs) > 0 { + if len(queryOptions.selectedAttrs) > 0 { // no need to fetch shares, we are only interested allowedActions and/or allowedRoles return collectionOfPermissions, nil } @@ -403,8 +410,11 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID } driveItems[storagespace.FormatResourceID(statResponse.GetInfo().GetId())] = *item + var permissionsCount int + if IsSpaceRoot(statResponse.GetInfo().GetId()) { - permissions, err := s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId()) + var permissions []libregraph.Permission + permissions, permissionsCount, err = s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId(), queryOptions.noValues) if err != nil { return collectionOfPermissions, err } @@ -438,14 +448,21 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID } for _, driveItem := range driveItems { - collectionOfPermissions.Value = append(collectionOfPermissions.Value, driveItem.Permissions...) + permissionsCount += len(driveItem.Permissions) + if !queryOptions.noValues { + collectionOfPermissions.Value = append(collectionOfPermissions.Value, driveItem.Permissions...) + } + } + + if queryOptions.count { + collectionOfPermissions.SetOdataCount(int32(permissionsCount)) } return collectionOfPermissions, nil } // ListSpaceRootPermissions handles ListPermissions request on project spaces -func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectedAttrs []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { +func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, queryOptions ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{} gatewayClient, err := s.gatewaySelector.Next() if err != nil { @@ -463,7 +480,7 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex } rootResourceID := space.GetRoot() - return s.ListPermissions(ctx, rootResourceID, false, selectedAttrs) // federated roles are not supported for spaces + return s.ListPermissions(ctx, rootResourceID, queryOptions) // federated roles are not supported for spaces } // DeletePermission deletes a permission from a drive item @@ -716,23 +733,17 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt return } - var listFederatedRoles bool - if odataReq.Query.Filter != nil { - if odataReq.Query.Filter.RawValue == federatedRolesODataFilter { - listFederatedRoles = true - } - } - - selectAttrs, err := odata.GetSelectValues(odataReq.Query) + var queryOptions ListPermissionsQueryOptions + queryOptions, err = api.getListPermissionsQueryOptions(odataReq) if err != nil { - api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest query options") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return } ctx := r.Context() - permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, itemID, listFederatedRoles, selectAttrs) + permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, itemID, queryOptions) if err != nil { errorcode.RenderError(w, r, err) return @@ -773,15 +784,16 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite return } - selected, err := odata.GetSelectValues(odataReq.Query) + var queryOptions ListPermissionsQueryOptions + queryOptions, err = api.getListPermissionsQueryOptions(odataReq) if err != nil { - api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest query options") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return } ctx := r.Context() - permissions, err := api.driveItemPermissionsService.ListSpaceRootPermissions(ctx, &driveID, selected) + permissions, err := api.driveItemPermissionsService.ListSpaceRootPermissions(ctx, &driveID, queryOptions) if err != nil { errorcode.RenderError(w, r, err) @@ -806,6 +818,40 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite render.JSON(w, r, permissions) } +func (api DriveItemPermissionsApi) getListPermissionsQueryOptions(odataReq *godata.GoDataRequest) (ListPermissionsQueryOptions, error) { + var listFederatedRoles bool + if odataReq.Query.Filter != nil { + if odataReq.Query.Filter.RawValue == federatedRolesODataFilter { + listFederatedRoles = true + } + } + + selectAttrs, err := odata.GetSelectValues(odataReq.Query) + if err != nil { + return ListPermissionsQueryOptions{}, err + } + + queryOptions := ListPermissionsQueryOptions{ + filterFederatedRoles: listFederatedRoles, + selectedAttrs: selectAttrs, + } + if odataReq.Query.Count != nil { + queryOptions.count = bool(*odataReq.Query.Count) + } + if odataReq.Query.Top != nil { + top := int(*odataReq.Query.Top) + switch { + case top != 0: + return ListPermissionsQueryOptions{}, err + case top == 0 && !queryOptions.count: + return ListPermissionsQueryOptions{}, err + default: + queryOptions.noValues = true + } + } + return queryOptions, nil +} + // DeletePermission handles DeletePermission requests func (api DriveItemPermissionsApi) DeletePermission(w http.ResponseWriter, r *http.Request) { _, itemID, err := GetDriveAndItemIDParam(r, &api.logger) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go index 4a7c908b6..52603c51f 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -385,7 +385,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil) gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil) gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) - permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{}) + permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, svc.ListPermissionsQueryOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -433,7 +433,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil) gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil) gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) - permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{}) + permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, svc.ListPermissionsQueryOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -472,7 +472,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil) gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil) gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) - permissions, err := service.ListPermissions(context.Background(), itemID, false, []string{}) + permissions, err := service.ListPermissions(context.Background(), itemID, svc.ListPermissionsQueryOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -508,7 +508,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient.On("ListShares", mock.Anything, mock.Anything).Return(listSharesResponse, nil) gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil) gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) - permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, []string{}) + permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, svc.ListPermissionsQueryOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -555,7 +555,7 @@ var _ = Describe("DriveItemPermissionsService", func() { gatewayClient.On("ListPublicShares", mock.Anything, mock.Anything).Return(listPublicSharesResponse, nil) statResponse.Info.Id = listSpacesResponse.StorageSpaces[0].Root gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil) - permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId, []string{}) + permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId, svc.ListPermissionsQueryOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) }) @@ -1268,8 +1268,7 @@ var _ = Describe("DriveItemPermissionsApi", func() { Expect(err).ToNot(HaveOccurred()) mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(func(ctx context.Context, itemid *provider.ResourceId, listFederatedRoles bool, selected []string) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { - Expect(listFederatedRoles).To(Equal(false)) + Return(func(ctx context.Context, itemid *provider.ResourceId, opt svc.ListPermissionsQueryOptions) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { Expect(storagespace.FormatResourceID(itemid)).To(Equal("1$2!3")) return libregraph.CollectionOfPermissionsWithAllowedValues{}, nil }).Once() diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 5578ee88b..ba07bc9dd 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -49,19 +49,20 @@ type BaseGraphService struct { availableRoles []*libregraph.UnifiedRoleDefinition } -func (g BaseGraphService) getSpaceRootPermissions(ctx context.Context, spaceID *storageprovider.StorageSpaceId) ([]libregraph.Permission, error) { +func (g BaseGraphService) getSpaceRootPermissions(ctx context.Context, spaceID *storageprovider.StorageSpaceId, countOnly bool) ([]libregraph.Permission, int, error) { gatewayClient, err := g.gatewaySelector.Next() if err != nil { g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - return nil, err + return nil, 0, err } space, err := utils.GetSpace(ctx, spaceID.GetOpaqueId(), gatewayClient) if err != nil { - return nil, errorcode.FromUtilsStatusCodeError(err) + return nil, 0, errorcode.FromUtilsStatusCodeError(err) } - return g.cs3SpacePermissionsToLibreGraph(ctx, space, APIVersion_1_Beta_1), nil + perm, count := g.cs3SpacePermissionsToLibreGraph(ctx, space, countOnly, APIVersion_1_Beta_1) + return perm, count, nil } func (g BaseGraphService) getDriveItem(ctx context.Context, ref *storageprovider.Reference) (*libregraph.DriveItem, error) { @@ -99,9 +100,9 @@ func (g BaseGraphService) CS3ReceivedOCMSharesToDriveItems(ctx context.Context, return cs3ReceivedOCMSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, g.availableRoles) } -func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, space *storageprovider.StorageSpace, apiVersion APIVersion) []libregraph.Permission { +func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, space *storageprovider.StorageSpace, countOnly bool, apiVersion APIVersion) ([]libregraph.Permission, int) { if space.Opaque == nil { - return nil + return nil, 0 } logger := g.logger.SubloggerWithRequestID(ctx) @@ -118,7 +119,12 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s } } if len(permissionsMap) == 0 { - return nil + return nil, 0 + } + + if countOnly { + // If we only need the count, we can return early + return nil, len(permissionsMap) } var permissionsExpirations map[string]*types.Timestamp @@ -219,7 +225,7 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s permissions = append(permissions, p) } - return permissions + return permissions, len(permissions) } func (g BaseGraphService) libreGraphPermissionFromCS3PublicShare(createdLink *link.PublicShare) (*libregraph.Permission, error) { @@ -1068,7 +1074,7 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st return nil, nil, err } - perms, err := g.getSpaceRootPermissions(ctx, resourceInfo.GetSpace().GetId()) + perms, _, err := g.getSpaceRootPermissions(ctx, resourceInfo.GetSpace().GetId(), false) if err != nil { return nil, nil, err } diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 49eca1390..490bf1358 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -20,10 +20,10 @@ import ( storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/go-chi/render" + libregraph "github.com/opencloud-eu/libre-graph-api-go" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/opencloud-eu/reva/v2/pkg/utils" - libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/pkg/errors" merrors "go-micro.dev/v4/errors" "golang.org/x/sync/errgroup" @@ -796,7 +796,7 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa }, } if expandPermissions { - drive.Root.Permissions = g.cs3SpacePermissionsToLibreGraph(ctx, space, apiVersion) + drive.Root.Permissions, _ = g.cs3SpacePermissionsToLibreGraph(ctx, space, false, apiVersion) } if space.SpaceType == _spaceTypeMountpoint {