From f98f7bd9e339c334013e83f54135def90d4fdbfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 8 Aug 2024 14:58:46 +0200 Subject: [PATCH] allow querying federated sharing roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/federated-user-roles.md | 5 + .../mocks/drive_item_permissions_provider.go | 30 ++-- .../service/v0/api_driveitem_permissions.go | 28 ++- .../v0/api_driveitem_permissions_test.go | 16 +- .../graph/pkg/service/v0/sharedwithme_test.go | 9 +- services/graph/pkg/service/v0/utils.go | 10 ++ services/graph/pkg/unifiedrole/unifiedrole.go | 81 ++++++++- .../graph/pkg/unifiedrole/unifiedrole_test.go | 36 +++- services/web/pkg/theme/theme.go | 8 + .../apiGraph/roleManagementEndpoint.feature | 170 +++++++++++++++++- 10 files changed, 354 insertions(+), 39 deletions(-) create mode 100644 changelog/unreleased/federated-user-roles.md diff --git a/changelog/unreleased/federated-user-roles.md b/changelog/unreleased/federated-user-roles.md new file mode 100644 index 00000000000..c56a1ef928b --- /dev/null +++ b/changelog/unreleased/federated-user-roles.md @@ -0,0 +1,5 @@ +Enhancement: allow querying federated user roles for sharing + +When listing permissions clients can now fetch the list of available federated sharing roles by sending a `GET /graph/v1beta1/drives/{driveid}/items/{itemid}/permissions?$filter=@libre.graph.permissions.roles.allowedValues/rolePermissions/any(p:contains(p/condition, '@Subject.UserType=="Federated"'))` request. Note that this is the only supported filter expression. Federated sharing roles will be omitted from requests without this filter. + +https://github.com/owncloud/ocis/pull/9765 diff --git a/services/graph/mocks/drive_item_permissions_provider.go b/services/graph/mocks/drive_item_permissions_provider.go index e28e9cb42be..22dda792299 100644 --- a/services/graph/mocks/drive_item_permissions_provider.go +++ b/services/graph/mocks/drive_item_permissions_provider.go @@ -294,9 +294,9 @@ func (_c *DriveItemPermissionsProvider_Invite_Call) RunAndReturn(run func(contex return _c } -// ListPermissions provides a mock function with given fields: ctx, itemID -func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { - ret := _m.Called(ctx, itemID) +// ListPermissions provides a mock function with given fields: ctx, itemID, listFederatedRoles, selectRoles +func (_m *DriveItemPermissionsProvider) ListPermissions(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + ret := _m.Called(ctx, itemID, listFederatedRoles, selectRoles) if len(ret) == 0 { panic("no return value specified for ListPermissions") @@ -304,17 +304,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) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { - return rf(ctx, itemID) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error)); ok { + return rf(ctx, itemID, listFederatedRoles, selectRoles) } - if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId) libregraph.CollectionOfPermissionsWithAllowedValues); ok { - r0 = rf(ctx, itemID) + if rf, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, bool, bool) libregraph.CollectionOfPermissionsWithAllowedValues); ok { + r0 = rf(ctx, itemID, listFederatedRoles, selectRoles) } else { r0 = ret.Get(0).(libregraph.CollectionOfPermissionsWithAllowedValues) } - if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId) error); ok { - r1 = rf(ctx, itemID) + if rf, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, bool, bool) error); ok { + r1 = rf(ctx, itemID, listFederatedRoles, selectRoles) } else { r1 = ret.Error(1) } @@ -330,13 +330,15 @@ type DriveItemPermissionsProvider_ListPermissions_Call struct { // ListPermissions is a helper method to define mock.On call // - ctx context.Context // - itemID *providerv1beta1.ResourceId -func (_e *DriveItemPermissionsProvider_Expecter) ListPermissions(ctx interface{}, itemID interface{}) *DriveItemPermissionsProvider_ListPermissions_Call { - return &DriveItemPermissionsProvider_ListPermissions_Call{Call: _e.mock.On("ListPermissions", ctx, itemID)} +// - listFederatedRoles bool +// - selectRoles bool +func (_e *DriveItemPermissionsProvider_Expecter) ListPermissions(ctx interface{}, itemID interface{}, listFederatedRoles interface{}, selectRoles interface{}) *DriveItemPermissionsProvider_ListPermissions_Call { + return &DriveItemPermissionsProvider_ListPermissions_Call{Call: _e.mock.On("ListPermissions", ctx, itemID, listFederatedRoles, selectRoles)} } -func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId)) *DriveItemPermissionsProvider_ListPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Run(run func(ctx context.Context, itemID *providerv1beta1.ResourceId, listFederatedRoles bool, selectRoles bool)) *DriveItemPermissionsProvider_ListPermissions_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), args[3].(bool)) }) return _c } @@ -346,7 +348,7 @@ func (_c *DriveItemPermissionsProvider_ListPermissions_Call) Return(_a0 libregra return _c } -func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_Call { +func (_c *DriveItemPermissionsProvider_ListPermissions_Call) RunAndReturn(run func(context.Context, *providerv1beta1.ResourceId, bool, bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error)) *DriveItemPermissionsProvider_ListPermissions_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 cca9e35656c..935a9e094f6 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -46,7 +46,7 @@ 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) (libregraph.CollectionOfPermissionsWithAllowedValues, error) + ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) ListSpaceRootPermissions(ctx context.Context, driveID *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) DeletePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) error DeleteSpaceRootPermission(ctx context.Context, driveID *storageprovider.ResourceId, permissionID string) error @@ -283,7 +283,7 @@ func createShareRequestToFederatedUser(user libregraph.User, resourceId *storage Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{ Type: userpb.UserType_USER_TYPE_FEDERATED, OpaqueId: user.GetId(), - Idp: providerInfo.Domain, // the domain is persisted in the grant as u:{opaqueid}:{domain} + Idp: *user.GetIdentities()[0].Issuer, // the domain is persisted in the grant as u:{opaqueid}:{domain} }}, }, RecipientMeshProvider: providerInfo, @@ -320,7 +320,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) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { +func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID *storageprovider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { collectionOfPermissions := libregraph.CollectionOfPermissionsWithAllowedValues{} gatewayClient, err := s.gatewaySelector.Next() if err != nil { @@ -347,6 +347,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID unifiedrole.GetApplicableRoleDefinitionsForActions( allowedActions, condition, + listFederatedRoles, false, ), ), @@ -358,6 +359,13 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID collectionOfPermissions.LibreGraphPermissionsRolesAllowedValues[i] = definition } + if selectRoles { + // drop the actions + collectionOfPermissions.LibreGraphPermissionsActionsAllowedValues = nil + // no need to fetch shares, we are only interested in the roles + return collectionOfPermissions, nil + } + driveItems := make(driveItemsByResourceID) if IsSpaceRoot(statResponse.GetInfo().GetId()) { permissions, err := s.getSpaceRootPermissions(ctx, statResponse.GetInfo().GetSpace().GetId()) @@ -408,7 +416,7 @@ func (s DriveItemPermissionsService) ListSpaceRootPermissions(ctx context.Contex } rootResourceID := space.GetRoot() - return s.ListPermissions(ctx, rootResourceID) + return s.ListPermissions(ctx, rootResourceID, false, false) // federated roles are not supported for spaces } // DeletePermission deletes a permission from a drive item @@ -626,9 +634,19 @@ 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 + } + + var selectRoles bool + if GetSelectParam(r) == "@libre.graph.permissions.roles.allowedValues" { + selectRoles = true + } + ctx := r.Context() - permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, &itemID) + permissions, err := api.driveItemPermissionsService.ListPermissions(ctx, &itemID, listFederatedRoles, selectRoles) if err != nil { errorcode.RenderError(w, r, err) return 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 3af2d99151c..467710d00ca 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -15,7 +15,6 @@ import ( collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/go-chi/chi/v5" . "github.com/onsi/ginkgo/v2" @@ -367,7 +366,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) + permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, false) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -414,7 +413,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) + permissions, err := driveItemPermissionsService.ListPermissions(context.Background(), itemID, false, false) Expect(err).ToNot(HaveOccurred()) Expect(len(permissions.LibreGraphPermissionsActionsAllowedValues)).ToNot(BeZero()) Expect(len(permissions.LibreGraphPermissionsRolesAllowedValues)).ToNot(BeZero()) @@ -1072,7 +1071,7 @@ var _ = Describe("DriveItemPermissionsApi", func() { onInvite := mockProvider.On("Invite", mock.Anything, mock.Anything, mock.Anything) - onInvite.Return(func(ctx context.Context, resourceID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { + onInvite.Return(func(ctx context.Context, resourceID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { return libregraph.Permission{}, errors.New("any") }).Once() @@ -1088,7 +1087,7 @@ var _ = Describe("DriveItemPermissionsApi", func() { Expect(err).ToNot(HaveOccurred()) onInvite := mockProvider.On("Invite", mock.Anything, mock.Anything, mock.Anything) - onInvite.Return(func(ctx context.Context, resourceID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { + onInvite.Return(func(ctx context.Context, resourceID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { Expect(storagespace.FormatResourceID(resourceID)).To(Equal("1$2!3")) return libregraph.Permission{}, nil }).Once() @@ -1109,7 +1108,7 @@ var _ = Describe("DriveItemPermissionsApi", func() { Expect(err).ToNot(HaveOccurred()) onInvite := mockProvider.On("SpaceRootInvite", mock.Anything, mock.Anything, mock.Anything) - onInvite.Return(func(ctx context.Context, driveID *storageprovider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { + onInvite.Return(func(ctx context.Context, driveID *provider.ResourceId, invite libregraph.DriveItemInvite) (libregraph.Permission, error) { Expect(storagespace.FormatResourceID(driveID)).To(Equal("1$2")) return libregraph.Permission{}, nil }).Once() @@ -1144,8 +1143,9 @@ var _ = Describe("DriveItemPermissionsApi", func() { inviteJson, err := json.Marshal(invite) Expect(err).ToNot(HaveOccurred()) - mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything). - Return(func(ctx context.Context, itemid *storageprovider.ResourceId) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + mockProvider.On("ListPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(func(ctx context.Context, itemid *provider.ResourceId, listFederatedRoles, selectRoles bool) (libregraph.CollectionOfPermissionsWithAllowedValues, error) { + Expect(listFederatedRoles).To(Equal(false)) Expect(storagespace.FormatResourceID(itemid)).To(Equal("1$2!3")) return libregraph.CollectionOfPermissionsWithAllowedValues{}, nil }).Once() diff --git a/services/graph/pkg/service/v0/sharedwithme_test.go b/services/graph/pkg/service/v0/sharedwithme_test.go index af5192a8be4..795ebefd897 100644 --- a/services/graph/pkg/service/v0/sharedwithme_test.go +++ b/services/graph/pkg/service/v0/sharedwithme_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" "google.golang.org/grpc" + "google.golang.org/protobuf/proto" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/shared" @@ -182,16 +183,16 @@ var _ = Describe("SharedWithMe", func() { } gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(func(_ context.Context, r *providerv1beta1.StatRequest, _ ...grpc.CallOption) (*providerv1beta1.StatResponse, error) { + // copy the response to avoid side effects + res := proto.Clone(statResponse).(*providerv1beta1.StatResponse) for _, share := range listReceivedSharesResponse.Shares { if share.Share.ResourceId != r.Ref.ResourceId { continue } - if statResponse.Info.Id == nil { - statResponse.Info.Id = share.Share.ResourceId - } + res.Info.Id = share.Share.ResourceId - return statResponse, nil + return res, nil } return nil, nil diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go index 2e954270847..9a1cd60ee93 100644 --- a/services/graph/pkg/service/v0/utils.go +++ b/services/graph/pkg/service/v0/utils.go @@ -72,6 +72,16 @@ func GetDriveAndItemIDParam(r *http.Request, logger *log.Logger) (storageprovide 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() diff --git a/services/graph/pkg/unifiedrole/unifiedrole.go b/services/graph/pkg/unifiedrole/unifiedrole.go index 4c373b532cc..c6275d4fc6c 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole.go +++ b/services/graph/pkg/unifiedrole/unifiedrole.go @@ -4,6 +4,7 @@ import ( "cmp" "errors" "slices" + "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" libregraph "github.com/owncloud/libre-graph-api-go" @@ -30,6 +31,18 @@ const ( UnifiedRoleManagerID = "312c0871-5ef7-4b3a-85b6-0e4074c64049" // UnifiedRoleSecureViewerID Unified role secure viewer id. UnifiedRoleSecureViewerID = "aa97fe03-7980-45ac-9e50-b325749fd7e6" + // UnifiedRoleFederatedViewerID Unified role federated viewer id. + UnifiedRoleFederatedViewerID = "be531789-063c-48bf-a9fe-857e6fbee7da" + // UnifiedRoleFederatedEditorID Unified role federated editor id. + UnifiedRoleFederatedEditorID = "36279a93-e4e3-4bbb-8a23-53b05b560963" + + // Wile the below conditions follow the SDDL syntax, they are not parsed anywhere. We use them as strings to + // represent the constraints that a role definition applies to. Fer the actual syntax, see the SDDL documentation + // at https://learn.microsoft.com/en-us/windows/win32/secauthz/security-descriptor-definition-language-for-conditional-aces-#conditional-expressions + + // Some roles apply to a specific type of resource, for example, a role that applies to a file or a folder. + // @Resource is the placeholder for the resource that the role is applied to + // .Root, .Folder and .File are facets of the driveItem resource that indicate the type of the resource if they are present. // UnifiedRoleConditionDrive defines constraint that matches a Driveroot/Spaceroot UnifiedRoleConditionDrive = "exists @Resource.Root" @@ -38,6 +51,19 @@ const ( // UnifiedRoleConditionFile defines a constraint that matches a DriveItem representing a File UnifiedRoleConditionFile = "exists @Resource.File" + // Some roles apply to a specific type of user, for example, a role that applies to a federated user. + // @Subject is the placeholder for the subject that the role is applied to. For sharing roles this is the user that the resource is shared with. + // .UserType is the type of the user: 'Member' for a member of the organization, 'Guest' for a guest user, 'Federated' for a federated user. + + // UnifiedRoleConditionFederatedUser defines a constraint that matches a federated user + UnifiedRoleConditionFederatedUser = "@Subject.UserType==\"Federated\"" + + // For federated sharing we need roles that combine the constraints for the resource and the user. + // UnifiedRoleConditionFileFederatedUser defines a constraint that matches a File and a federated user + UnifiedRoleConditionFileFederatedUser = UnifiedRoleConditionFile + " && " + UnifiedRoleConditionFederatedUser + // UnifiedRoleConditionFolderFederatedUser defines a constraint that matches a Folder and a federated user + UnifiedRoleConditionFolderFederatedUser = UnifiedRoleConditionFolder + " && " + UnifiedRoleConditionFederatedUser + DriveItemPermissionsCreate = "libre.graph/driveItem/permissions/create" DriveItemChildrenCreate = "libre.graph/driveItem/children/create" DriveItemStandardDelete = "libre.graph/driveItem/standard/delete" @@ -258,6 +284,48 @@ func NewSecureViewerUnifiedRole() *libregraph.UnifiedRoleDefinition { } } +// NewFederatedViewerUnifiedRole creates a federated viewer role +func NewFederatedViewerUnifiedRole() *libregraph.UnifiedRoleDefinition { + r := conversions.NewViewerRole() + return &libregraph.UnifiedRoleDefinition{ + Id: proto.String(UnifiedRoleFederatedViewerID), + Description: proto.String("View and download."), + DisplayName: displayName(r), + RolePermissions: []libregraph.UnifiedRolePermission{ + { + AllowedResourceActions: convert(r), + Condition: proto.String(UnifiedRoleConditionFileFederatedUser), + }, + { + AllowedResourceActions: convert(r), + Condition: proto.String(UnifiedRoleConditionFolderFederatedUser), + }, + }, + LibreGraphWeight: proto.Int32(0), + } +} + +// NewFederatedEditorUnifiedRole creates a federated editor role +func NewFederatedEditorUnifiedRole() *libregraph.UnifiedRoleDefinition { + r := conversions.NewEditorRole() + return &libregraph.UnifiedRoleDefinition{ + Id: proto.String(UnifiedRoleFederatedEditorID), + Description: proto.String("View, download and edit."), + DisplayName: displayName(r), + RolePermissions: []libregraph.UnifiedRolePermission{ + { + AllowedResourceActions: convert(r), + Condition: proto.String(UnifiedRoleConditionFileFederatedUser), + }, + { + AllowedResourceActions: convert(r), + Condition: proto.String(UnifiedRoleConditionFolderFederatedUser), + }, + }, + LibreGraphWeight: proto.Int32(0), + } +} + // NewUnifiedRoleFromID returns a unified role definition from the provided id func NewUnifiedRoleFromID(id string) (*libregraph.UnifiedRoleDefinition, error) { for _, definition := range GetBuiltinRoleDefinitionList() { @@ -281,12 +349,14 @@ func GetBuiltinRoleDefinitionList() []*libregraph.UnifiedRoleDefinition { NewEditorLiteUnifiedRole(), NewManagerUnifiedRole(), NewSecureViewerUnifiedRole(), + NewFederatedViewerUnifiedRole(), + NewFederatedEditorUnifiedRole(), } } // GetApplicableRoleDefinitionsForActions returns a list of role definitions // that match the provided actions and constraints -func GetApplicableRoleDefinitionsForActions(actions []string, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition { +func GetApplicableRoleDefinitionsForActions(actions []string, constraints string, listFederatedRoles, descending bool) []*libregraph.UnifiedRoleDefinition { builtin := GetBuiltinRoleDefinitionList() definitions := make([]*libregraph.UnifiedRoleDefinition, 0, len(builtin)) @@ -294,7 +364,14 @@ func GetApplicableRoleDefinitionsForActions(actions []string, constraints string var definitionMatch bool for _, permission := range definition.GetRolePermissions() { - if permission.GetCondition() != constraints { + // this is a dirty comparison because we are not really parsing the SDDL, but as long as we && the conditions we are good + isFederatedRole := strings.Contains(permission.GetCondition(), UnifiedRoleConditionFederatedUser) + switch { + case !strings.Contains(permission.GetCondition(), constraints): + continue + case listFederatedRoles && !isFederatedRole: + continue + case !listFederatedRoles && isFederatedRole: continue } diff --git a/services/graph/pkg/unifiedrole/unifiedrole_test.go b/services/graph/pkg/unifiedrole/unifiedrole_test.go index 8cfe6093920..e9195509096 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole_test.go +++ b/services/graph/pkg/unifiedrole/unifiedrole_test.go @@ -1,7 +1,6 @@ package unifiedrole_test import ( - "fmt" "slices" rConversions "github.com/cs3org/reva/v2/pkg/conversions" @@ -114,9 +113,9 @@ var _ = Describe("unifiedroles", func() { } DescribeTable("GetApplicableRoleDefinitionsForActions", - func(givenActions []string, constraints string, expectedDefinitions []*libregraph.UnifiedRoleDefinition) { + func(givenActions []string, constraints string, listFederatedRoles bool, expectedDefinitions []*libregraph.UnifiedRoleDefinition) { - generatedDefinitions := unifiedrole.GetApplicableRoleDefinitionsForActions(givenActions, constraints, false) + generatedDefinitions := unifiedrole.GetApplicableRoleDefinitionsForActions(givenActions, constraints, listFederatedRoles, false) Expect(len(generatedDefinitions)).To(Equal(len(expectedDefinitions))) @@ -137,6 +136,7 @@ var _ = Describe("unifiedroles", func() { "ViewerUnifiedRole", rolesToAction(unifiedrole.NewViewerUnifiedRole()), unifiedrole.UnifiedRoleConditionFolder, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), @@ -147,16 +147,38 @@ var _ = Describe("unifiedroles", func() { "ViewerUnifiedRole | share", rolesToAction(unifiedrole.NewViewerUnifiedRole()), unifiedrole.UnifiedRoleConditionFile, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), }, ), + Entry( + "FederatedViewerUnifiedRole | share", + rolesToAction(unifiedrole.NewFederatedViewerUnifiedRole()), + unifiedrole.UnifiedRoleConditionFile, + true, + []*libregraph.UnifiedRoleDefinition{ + unifiedrole.NewFederatedViewerUnifiedRole(), + }, + ), + Entry( + "FederatedEditorUnifiedRole | share", + rolesToAction(unifiedrole.NewFederatedEditorUnifiedRole()), + unifiedrole.UnifiedRoleConditionFile, + true, + []*libregraph.UnifiedRoleDefinition{ + unifiedrole.NewFederatedViewerUnifiedRole(), + unifiedrole.NewFederatedEditorUnifiedRole(), + }, + ), + Entry( "NewFileEditorUnifiedRole", rolesToAction(unifiedrole.NewFileEditorUnifiedRole()), unifiedrole.UnifiedRoleConditionFile, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), @@ -168,6 +190,7 @@ var _ = Describe("unifiedroles", func() { "NewEditorUnifiedRole", rolesToAction(unifiedrole.NewEditorUnifiedRole()), unifiedrole.UnifiedRoleConditionFolder, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), @@ -180,6 +203,7 @@ var _ = Describe("unifiedroles", func() { "GetBuiltinRoleDefinitionList", rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...), unifiedrole.UnifiedRoleConditionFile, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), @@ -191,6 +215,7 @@ var _ = Describe("unifiedroles", func() { "GetBuiltinRoleDefinitionList", rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...), unifiedrole.UnifiedRoleConditionFolder, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewViewerUnifiedRole(), @@ -203,6 +228,7 @@ var _ = Describe("unifiedroles", func() { "GetBuiltinRoleDefinitionList", rolesToAction(unifiedrole.GetBuiltinRoleDefinitionList()...), unifiedrole.UnifiedRoleConditionDrive, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSpaceViewerUnifiedRole(), unifiedrole.NewSpaceEditorUnifiedRole(), @@ -214,6 +240,7 @@ var _ = Describe("unifiedroles", func() { "single", []string{unifiedrole.DriveItemQuotaRead}, unifiedrole.UnifiedRoleConditionFile, + false, []*libregraph.UnifiedRoleDefinition{}, ), @@ -221,6 +248,7 @@ var _ = Describe("unifiedroles", func() { "mixed", append(rolesToAction(unifiedrole.NewEditorLiteUnifiedRole()), unifiedrole.DriveItemQuotaRead), unifiedrole.UnifiedRoleConditionFolder, + false, []*libregraph.UnifiedRoleDefinition{ unifiedrole.NewSecureViewerUnifiedRole(), unifiedrole.NewEditorLiteUnifiedRole(), @@ -233,7 +261,7 @@ var _ = Describe("unifiedroles", func() { var newUnifiedRoleFromIDEntries []TableEntry attachEntry := func(name, id string, definition *libregraph.UnifiedRoleDefinition, errors bool) { e := Entry( - fmt.Sprintf("%s", name), + name, id, definition, errors, diff --git a/services/web/pkg/theme/theme.go b/services/web/pkg/theme/theme.go index a6f8a57b900..6d93078bc5b 100644 --- a/services/web/pkg/theme/theme.go +++ b/services/web/pkg/theme/theme.go @@ -49,6 +49,14 @@ var themeDefaults = KV{ "label": "UnifiedRoleSecureView", "iconName": "shield", }, + unifiedrole.UnifiedRoleFederatedViewerID: KV{ + "label": "UnifiedRoleFederatedViewer", + "iconName": "eye", + }, + unifiedrole.UnifiedRoleFederatedEditorID: KV{ + "label": "UnifiedRoleFederatedEditor", + "iconName": "pencil", + }, }, }, } diff --git a/tests/acceptance/features/apiGraph/roleManagementEndpoint.feature b/tests/acceptance/features/apiGraph/roleManagementEndpoint.feature index 2071ee413b6..743ba56e757 100644 --- a/tests/acceptance/features/apiGraph/roleManagementEndpoint.feature +++ b/tests/acceptance/features/apiGraph/roleManagementEndpoint.feature @@ -14,8 +14,8 @@ Feature: permissions role definitions """ { "type": "array", - "maxItems": 8, - "minItems": 8, + "maxItems": 10, + "minItems": 10, "uniqueItems": true, "items": { "oneOf": [ @@ -510,6 +510,172 @@ Feature: permissions role definitions } } } + }, + { + "type": "object", + "required": [ + "@libre.graph.weight", + "description", + "displayName", + "id", + "rolePermissions" + ], + "properties": { + "@libre.graph.weight": { + "const": 0 + }, + "description": { + "const": "View and download." + }, + "displayName": { + "const": "Can view" + }, + "id": { + "const": "be531789-063c-48bf-a9fe-857e6fbee7da" + }, + "rolePermissions": { + "type": "array", + "maxItems": 2, + "minItems": 2, + "uniqueItems": true, + "items": { + "oneOf": [ + { + "type": "object", + "required": [ + "allowedResourceActions", + "condition" + ], + "properties": { + "allowedResourceActions": { + "const": [ + "libre.graph/driveItem/path/read", + "libre.graph/driveItem/quota/read", + "libre.graph/driveItem/content/read", + "libre.graph/driveItem/children/read", + "libre.graph/driveItem/deleted/read", + "libre.graph/driveItem/basic/read" + ] + }, + "condition": { + "const": "exists @Resource.File \u0026\u0026 @Subject.UserType==\"Federated\"" + } + } + }, + { + "type": "object", + "required": [ + "allowedResourceActions", + "condition" + ], + "properties": { + "allowedResourceActions": { + "const": [ + "libre.graph/driveItem/path/read", + "libre.graph/driveItem/quota/read", + "libre.graph/driveItem/content/read", + "libre.graph/driveItem/children/read", + "libre.graph/driveItem/deleted/read", + "libre.graph/driveItem/basic/read" + ] + }, + "condition": { + "const": "exists @Resource.Folder \u0026\u0026 @Subject.UserType==\"Federated\"" + } + } + } + ] + } + } + } + }, + { + "type": "object", + "required": [ + "@libre.graph.weight", + "description", + "displayName", + "id", + "rolePermissions" + ], + "properties": { + "@libre.graph.weight": { + "const": 0 + }, + "description": { + "const": "View, download and edit." + }, + "displayName": { + "const": "Can edit" + }, + "id": { + "const": "36279a93-e4e3-4bbb-8a23-53b05b560963" + }, + "rolePermissions": { + "type": "array", + "maxItems": 2, + "minItems": 2, + "uniqueItems": true, + "items": { + "oneOf": [ + { + "type": "object", + "required": [ + "allowedResourceActions", + "condition" + ], + "properties": { + "allowedResourceActions": { + "const": [ + "libre.graph/driveItem/children/create", + "libre.graph/driveItem/standard/delete", + "libre.graph/driveItem/path/read", + "libre.graph/driveItem/quota/read", + "libre.graph/driveItem/content/read", + "libre.graph/driveItem/upload/create", + "libre.graph/driveItem/children/read", + "libre.graph/driveItem/deleted/read", + "libre.graph/driveItem/path/update", + "libre.graph/driveItem/deleted/update", + "libre.graph/driveItem/basic/read" + ] + }, + "condition": { + "const": "exists @Resource.File \u0026\u0026 @Subject.UserType==\"Federated\"" + } + } + }, + { + "type": "object", + "required": [ + "allowedResourceActions", + "condition" + ], + "properties": { + "allowedResourceActions": { + "const": [ + "libre.graph/driveItem/children/create", + "libre.graph/driveItem/standard/delete", + "libre.graph/driveItem/path/read", + "libre.graph/driveItem/quota/read", + "libre.graph/driveItem/content/read", + "libre.graph/driveItem/upload/create", + "libre.graph/driveItem/children/read", + "libre.graph/driveItem/deleted/read", + "libre.graph/driveItem/path/update", + "libre.graph/driveItem/deleted/update", + "libre.graph/driveItem/basic/read" + ] + }, + "condition": { + "const": "exists @Resource.Folder \u0026\u0026 @Subject.UserType==\"Federated\"" + } + } + } + ] + } + } + } } ] }