diff --git a/services/graph/mocks/drive_item_permissions_provider.go b/services/graph/mocks/drive_item_permissions_provider.go index b5f6d67ee..42f8f9793 100644 --- a/services/graph/mocks/drive_item_permissions_provider.go +++ b/services/graph/mocks/drive_item_permissions_provider.go @@ -353,9 +353,9 @@ func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run fu return _c } -// ListSpaceRootPermissions provides a mock function with given fields: ctx, driveID -func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { - ret := _m.Called(ctx, driveID) +// ListSpaceRootPermissions provides a mock function with given fields: ctx, driveID, selectRoles +func (_m *DriveItemPermissionsProvider) ListSpaceRootPermissions(ctx context.Context, driveID *providerv1beta1.ResourceId, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + ret := _m.Called(ctx, driveID, selectRoles) if len(ret) == 0 { panic("no return value specified for ListSpaceRootPermissions") @@ -363,17 +363,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) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { - return rf(ctx, driveID) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { + return rf(ctx, driveID, selectRoles) } - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId) libregraph.CollectionOfPermissionsWithAllowedValues); ok { - r0 = rf(ctx, driveID) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool) libregraph.CollectionOfPermissionsWithAllowedValues); ok { + r0 = rf(ctx, driveID, selectRoles) } else { r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues) } - if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId) error); ok { - r1 = rf(ctx, driveID) + if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, bool) error); ok { + r1 = rf(ctx, driveID, selectRoles) } else { r1 = ret.Error(1) } @@ -389,13 +389,14 @@ type DriveItemPermissionsProvider_ListSpaceRootPermissions_Call struct { // ListSpaceRootPermissions is a helper method to define mock.On call // - ctx context.Context // - driveID *providerv1beta1.ResourceId -func (_e *DriveItemPermissionsProvider_Expecter) ListSpaceRootPermissions(ctx interface{}, driveID interface{}) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { - return &DriveItemPermissionsProvider_ListSpaceRootPermissions_Call{Call: _e.mock.On("ListSpaceRootPermissions", ctx, driveID)} +// - selectRoles bool +func (_e *DriveItemPermissionsProvider_Expecter) ListSpaceRootPermissions(ctx interface{}, driveID interface{}, selectRoles interface{}) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { + return &DriveItemPermissionsProvider_ListSpaceRootPermissions_Call{Call: _e.mock.On("ListSpaceRootPermissions", ctx, driveID, selectRoles)} } -func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Run(run func(ctx context.Context, driveID *providerv1beta1.ResourceId, selectRoles bool)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId)) + run(args[0].(context.Context), args[1].(*providerv1beta1.ResourceId), args[2].(bool)) }) return _c } @@ -405,7 +406,7 @@ func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) Return(_a0 return _c } -func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListSpaceRootPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, bool) (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 07bc75170..19c1fdbae 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -6,7 +6,9 @@ import ( "net/http" "net/url" "slices" + "strings" + "github.com/CiscoM31/godata" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -40,8 +42,9 @@ import ( ) const ( - invalidIdMsg = "invalid driveID or itemID" - parseDriveIDErrMsg = "could not parse driveID" + invalidIdMsg = "invalid driveID or itemID" + parseDriveIDErrMsg = "could not parse driveID" + federatedRolesODataFilter = "@libre.graph.permissions.roles.allowedValues/rolePermissions/any(p:contains(p/condition, '@Subject.UserType==\"Federated\"'))" ) // DriveItemPermissionsProvider contains the methods related to handling permissions on drive items @@ -49,7 +52,7 @@ 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, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) - ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) + ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectRoles bool) (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) @@ -438,7 +441,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID } // ListSpaceRootPermissions handles ListPermissions request on project spaces -func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { +func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{} gatewayClient, err := s.gatewaySelector.Next() if err != nil { @@ -456,7 +459,7 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex } rootResourceID := space.GetRoot() - return s.ListPermissions(ctx, rootResourceID, false, false) // federated roles are not supported for spaces + return s.ListPermissions(ctx, rootResourceID, false, selectRoles) // federated roles are not supported for spaces } // DeletePermission deletes a permission from a drive item @@ -701,14 +704,26 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt return } - var listFederatedRoles bool - if GetFilterParam(r) == "@libre.graph.permissions.roles.allowedValues/rolePermissions/any(p:contains(p/condition, '@Subject.UserType==\"Federated\"'))" { - listFederatedRoles = true + sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") + odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) + if err != nil { + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return } - var selectRoles bool - if GetSelectParam(r) == "@libre.graph.permissions.roles.allowedValues" { - selectRoles = true + var listFederatedRoles bool + if odataReq.Query.Filter != nil { + if odataReq.Query.Filter.RawValue == federatedRolesODataFilter { + listFederatedRoles = true + } + } + + selectRoles, err := api.listPermissionsQuerySelectValues(odataReq.Query) + if err != nil { + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return } ctx := r.Context() @@ -746,8 +761,23 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite return } + sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") + odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) + if err != nil { + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + + selectRoles, err := api.listPermissionsQuerySelectValues(odataReq.Query) + if err != nil { + api.logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("Error parsing ListPermissionRequest: query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + ctx := r.Context() - permissions, err := api.driveItemPermissionsService.ListSpaceRootPermissions(ctx, &driveID) + permissions, err := api.driveItemPermissionsService.ListSpaceRootPermissions(ctx, &driveID, selectRoles) if err != nil { errorcode.RenderError(w, r, err) @@ -903,3 +933,21 @@ func (api DriveItemPermissionsApi) UpdateSpaceRootPermission(w http.ResponseWrit render.Status(r, http.StatusOK) render.JSON(w, r, &updatedPermission) } + +func (api DriveItemPermissionsApi) listPermissionsQuerySelectValues(odataQuery *godata.GoDataQuery) (bool, error) { + if odataQuery.Select != nil { + for _, item := range odataQuery.Select.SelectItems { + if len(item.Segments) != 1 { + api.logger.Debug().Msg("Error parsing ListPermissionRequest: unsupported select item") + return false, errorcode.New(errorcode.InvalidRequest, "unsupported select item") + } + // for now we only support the select for the roles + if item.Segments[0].Value != "@libre.graph.permissions.roles.allowedValues" { + api.logger.Debug().Msg("Error parsing ListPermissionRequest: unsupported select item") + return false, errorcode.New(errorcode.InvalidRequest, "unsupported select item") + } + return true, nil + } + } + return false, nil +} 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 d88687b9d..7228e96a4 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -20,8 +20,8 @@ import ( "github.com/go-chi/chi/v5" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/opencloud-eu/opencloud/services/graph/pkg/config" libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" "google.golang.org/grpc" @@ -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) + permissions, err := driveItemPermissionsService.ListSpaceRootPermissions(context.Background(), driveId, false) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) }) diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go index 5b98d9634..7e85d9dad 100644 --- a/services/graph/pkg/service/v0/utils.go +++ b/services/graph/pkg/service/v0/utils.go @@ -13,9 +13,9 @@ import ( collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + libregraph "github.com/opencloud-eu/libre-graph-api-go" "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" "golang.org/x/sync/errgroup" "github.com/opencloud-eu/opencloud/pkg/log" @@ -73,16 +73,6 @@ func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (*storageprovid return &driveID, &itemID, nil } -// GetFilterParam returns the $filter query parameter from the request. If you need to parse the filter use godata.ParseRequest -func GetFilterParam(r *http.Request) string { - return r.URL.Query().Get("$filter") -} - -// GetSelectParam returns the $select query parameter from the request. If you need to parse the select filter use godata.ParseRequest -func GetSelectParam(r *http.Request) string { - return r.URL.Query().Get("$select") -} - // GetGatewayClient returns a gateway client from the gatewaySelector. func (g Graph) GetGatewayClient(w http.ResponseWriter, r *http.Request) (gateway.GatewayAPIClient, bool) { gatewayClient, err := g.gatewaySelector.Next()