enhancement: only use allowed roles for the graph service

changelog/unreleased/enhancement-unified-roles-management.md

@@ -1,6 +1,8 @@
 Enhancement: Unified Roles Management
 
 Improved management of unified roles with the introduction of default enabled/disabled states and a new command for listing available roles.
+It is important to note that a disabled role does not lose previously assigned permissions;
+it only means that the role is not available for new assignments.
 
 The following roles are now enabled by default:
 

services/graph/pkg/command/unified_roles.go

@@ -44,7 +44,8 @@ func unifiedRolesStatus(cfg *config.Config) *cli.Command {
 
 			var data [][]string
 
-			for _, definition := range unifiedrole.GetDefinitions(unifiedrole.RoleFilterAll()) {
+			// fixMe: should we use all definitions?
+			for _, definition := range unifiedrole.GetRoles(unifiedrole.RoleFilterAll()) {
 				data = append(data, []string{"", definition.GetId(), definition.GetDescription()})
 			}
 

services/graph/pkg/config/defaults/defaultconfig.go

@@ -177,7 +177,7 @@ func EnsureDefaults(cfg *config.Config) {
 
 	// set default roles, if no roles are defined, we need to take care and provide all the default roles
 	if len(cfg.UnifiedRoles.AvailableRoles) == 0 {
-		for _, definition := range unifiedrole.GetDefinitions(
+		for _, definition := range unifiedrole.GetRoles(
 			// filter out the roles that are disabled by default
 			unifiedrole.RoleFilterInvert(unifiedrole.RoleFilterIDs(_disabledByDefaultUnifiedRoleRoleIDs...)),
 		) {

services/graph/pkg/config/parser/parse.go

@@ -79,9 +79,9 @@ func Validate(cfg *config.Config) error {
 
 		for _, uid := range cfg.UnifiedRoles.AvailableRoles {
 			// check if the role is known
-			if len(unifiedrole.GetDefinitions(unifiedrole.RoleFilterIDs(uid))) == 0 {
+			if len(unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(uid))) == 0 {
 				// collect all possible errors to return them all at once
-				err = errors.Join(err, fmt.Errorf("%w: %s", unifiedrole.ErrUnknownUnifiedRole, uid))
+				err = errors.Join(err, fmt.Errorf("%w: %s", unifiedrole.ErrUnknownRole, uid))
 			}
 		}
 

services/graph/pkg/service/v0/api_driveitem_permissions.go

@@ -106,10 +106,10 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 	for _, roleID := range invite.GetRoles() {
 		// only allow roles that are enabled in the config
 		if !slices.Contains(s.config.UnifiedRoles.AvailableRoles, roleID) {
-			return libregraph.Permission{}, unifiedrole.ErrUnknownUnifiedRole
+			return libregraph.Permission{}, unifiedrole.ErrUnknownRole
 		}
 
-		role, err := unifiedrole.GetDefinition(unifiedrole.RoleFilterIDs(roleID))
+		role, err := unifiedrole.GetRole(unifiedrole.RoleFilterIDs(roleID))
 		if err != nil {
 			s.logger.Debug().Err(err).Interface("role", invite.GetRoles()[0]).Msg("unable to convert requested role")
 			return libregraph.Permission{}, err
@@ -129,7 +129,8 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 	cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
 
 	permission := &libregraph.Permission{}
-	if role := unifiedrole.CS3ResourcePermissionsToDefinition(cs3ResourcePermissions, condition); role != nil {
+	availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(s.config.UnifiedRoles.AvailableRoles...))
+	if role := unifiedrole.CS3ResourcePermissionsToRole(availableRoles, cs3ResourcePermissions, condition); role != nil {
 		permission.Roles = []string{role.GetId()}
 	}
 
@@ -226,7 +227,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 	} else if IsSpaceRoot(statResponse.GetInfo().GetId()) {
 		// permissions on a space root are not handled by a share manager so
 		// they don't get a share-id
-		permission.SetId(identitySetToSpacePermissionID(permission.GetGrantedToV2()))
+		permission.SetId(identitySetToSpacePermissionID(permission.GetGrantedToV2(), s.config.UnifiedRoles.AvailableRoles))
 	}
 
 	if expiration != nil {
@@ -351,6 +352,7 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID
 		LibreGraphPermissionsActionsAllowedValues: allowedActions,
 		LibreGraphPermissionsRolesAllowedValues: conversions.ToValueSlice(
 			unifiedrole.GetRolesByPermissions(
+				unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(s.config.UnifiedRoles.AvailableRoles...)),
 				allowedActions,
 				condition,
 				listFederatedRoles,

services/graph/pkg/service/v0/api_driveitem_permissions_test.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"slices"
 	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
@@ -54,6 +55,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 				OpaqueId: "user",
 			},
 		}
+		cache        identity.IdentityCache
 		statResponse *provider.StatResponse
 		driveItemId  *provider.ResourceId
 		ctx          context.Context
@@ -66,7 +68,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 		gatewaySelector = mocks.NewSelectable[gateway.GatewayAPIClient](GinkgoT())
 		gatewaySelector.On("Next").Return(gatewayClient, nil)
 
-		cache := identity.NewIdentityCache(identity.IdentityCacheWithGatewaySelector(gatewaySelector))
+		cache = identity.NewIdentityCache(identity.IdentityCacheWithGatewaySelector(gatewaySelector))
 
 		cfg := defaults.FullDefaultConfig()
 		service, err := svc.NewDriveItemPermissionsService(logger, gatewaySelector, cache, cfg)
@@ -255,9 +257,17 @@ var _ = Describe("DriveItemPermissionsService", func() {
 		})
 
 		It("fails with unknown or disable role", func() {
+			cfg := defaults.FullDefaultConfig()
+			slices.DeleteFunc(cfg.UnifiedRoles.AvailableRoles, func(s string) bool {
+				// SecureViewer is enabled in ci, we need to remove it in the unit test
+				return s != unifiedrole.UnifiedRoleSecureViewerID
+			})
+			service, err := svc.NewDriveItemPermissionsService(log.NewLogger(), gatewaySelector, cache, cfg)
+			Expect(err).ToNot(HaveOccurred())
+
 			driveItemInvite.Roles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleSecureViewerID}
-			_, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
+			_, err = service.Invite(context.Background(), driveItemId, driveItemInvite)
-			Expect(err).To(MatchError(unifiedrole.ErrUnknownUnifiedRole))
+			Expect(err).To(MatchError(unifiedrole.ErrUnknownRole))
 		})
 	})
 	Describe("SpaceRootInvite", func() {

services/graph/pkg/service/v0/base.go

@@ -85,7 +85,8 @@ func (g BaseGraphService) CS3ReceivedSharesToDriveItems(ctx context.Context, rec
 		return nil, err
 	}
 
-	return cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares)
+	availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...))
+	return cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, receivedShares, availableRoles)
 }
 
 func (g BaseGraphService) CS3ReceivedOCMSharesToDriveItems(ctx context.Context, receivedShares []*ocm.ReceivedShare) ([]libregraph.DriveItem, error) {
@@ -183,7 +184,7 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s
 			} else {
 				identitySet.SetUser(identity)
 			}
-			p.SetId(identitySetToSpacePermissionID(identitySet))
+			p.SetId(identitySetToSpacePermissionID(identitySet, g.config.UnifiedRoles.AvailableRoles))
 			p.SetGrantedToV2(identitySet)
 		}
 
@@ -191,10 +192,15 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s
 			p.SetExpirationDateTime(time.Unix(int64(exp.GetSeconds()), int64(exp.GetNanos())))
 		}
 
-		if role := unifiedrole.CS3ResourcePermissionsToDefinition(perm, unifiedrole.UnifiedRoleConditionDrive); role != nil {
+		availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...))
+		if role := unifiedrole.CS3ResourcePermissionsToRole(
+			availableRoles,
+			perm,
+			unifiedrole.UnifiedRoleConditionDrive,
+		); role != nil {
 			switch apiVersion {
 			case APIVersion_1:
-				if r := unifiedrole.GetLegacyDefinitionName(*role); r != "" {
+				if r := unifiedrole.GetLegacyRoleName(*role); r != "" {
 					p.SetRoles([]string{r})
 				}
 			case APIVersion_1_Beta_1:
@@ -392,7 +398,9 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
 	if share.GetCtime() != nil {
 		perm.SetCreatedDateTime(cs3TimestampToTime(share.GetCtime()))
 	}
-	role := unifiedrole.CS3ResourcePermissionsToDefinition(
+	// fixMe: should we use all roles?
+	role := unifiedrole.CS3ResourcePermissionsToRole(
+		unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)),
 		share.GetPermissions().GetPermissions(),
 		roleCondition,
 	)
@@ -695,7 +703,7 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri
 	var permissionsUpdated, ok bool
 	if roles, ok = newPermission.GetRolesOk(); ok && len(roles) > 0 {
 		for _, roleID := range roles {
-			role, err := unifiedrole.GetDefinition(unifiedrole.RoleFilterIDs(roleID))
+			role, err := unifiedrole.GetRole(unifiedrole.RoleFilterIDs(roleID))
 			if err != nil {
 				g.logger.Debug().Err(err).Interface("role", role).Msg("unable to convert requested role")
 				return nil, err

services/graph/pkg/service/v0/rolemanagement.go

@@ -14,7 +14,7 @@ import (
 // GetRoleDefinitions a list of permission roles than can be used when sharing with users or groups
 func (g Graph) GetRoleDefinitions(w http.ResponseWriter, r *http.Request) {
 	render.Status(r, http.StatusOK)
-	render.JSON(w, r, unifiedrole.GetDefinitions(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)))
+	render.JSON(w, r, unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)))
 }
 
 // GetRoleDefinition a permission role than can be used when sharing with users or groups
@@ -26,7 +26,7 @@ func (g Graph) GetRoleDefinition(w http.ResponseWriter, r *http.Request) {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping role id failed")
 		return
 	}
-	role, err := unifiedrole.GetDefinition(unifiedrole.RoleFilterIDs(roleID))
+	role, err := unifiedrole.GetRole(unifiedrole.RoleFilterIDs(roleID))
 	if err != nil {
 		logger.Debug().Str("roleID", roleID).Msg("could not get role: not found")
 		errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, err.Error())

services/graph/pkg/service/v0/sharedwithme.go

@@ -10,6 +10,7 @@ import (
 	libregraph "github.com/owncloud/libre-graph-api-go"
 
 	"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 )
 
 // ListSharedWithMe lists the files shared with the current user.
@@ -39,7 +40,8 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
 		g.logger.Error().Err(err).Msg("listing shares failed")
 		return nil, err
 	}
-	driveItems, err := cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedSharesResponse.GetShares())
+	availableRoles := unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...))
+	driveItems, err := cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedSharesResponse.GetShares(), availableRoles)
 	if err != nil {
 		g.logger.Error().Err(err).Msg("could not convert received shares to drive items")
 		return nil, err

services/graph/pkg/service/v0/utils.go

@@ -140,7 +140,7 @@ func groupIdToIdentity(ctx context.Context, cache identity.IdentityCache, groupI
 // As permissions on space to not map to a cs3 share we need something else of the ids. So we just
 // construct the id for the id of the user or group that the permission applies to and prefix that
 // with a "u:" for userids and "g:" for group ids.
-func identitySetToSpacePermissionID(identitySet libregraph.SharePointIdentitySet) (id string) {
+func identitySetToSpacePermissionID(identitySet libregraph.SharePointIdentitySet, allowedRoleIDs []string) (id string) {
 	switch {
 	case identitySet.HasUser():
 		id = "u:" + identitySet.User.GetId()
@@ -154,7 +154,9 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
 	logger *log.Logger,
 	gatewayClient gateway.GatewayAPIClient,
 	identityCache identity.IdentityCache,
-	receivedShares []*collaboration.ReceivedShare) ([]libregraph.DriveItem, error) {
+	receivedShares []*collaboration.ReceivedShare,
+	availableRoles []*libregraph.UnifiedRoleDefinition,
+) ([]libregraph.DriveItem, error) {
 
 	group := new(errgroup.Group)
 	// Set max concurrency
@@ -196,7 +198,7 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
 				return errCode
 			}
 
-			driveItem, err := fillDriveItemPropertiesFromReceivedShare(ctx, logger, identityCache, receivedShares, shareStat.GetInfo())
+			driveItem, err := fillDriveItemPropertiesFromReceivedShare(ctx, logger, identityCache, receivedShares, shareStat.GetInfo(), availableRoles)
 			if err != nil {
 				return err
 			}
@@ -333,7 +335,7 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context,
 
 func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.Logger,
 	identityCache identity.IdentityCache, receivedShares []*collaboration.ReceivedShare,
-	resourceInfo *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) {
+	resourceInfo *storageprovider.ResourceInfo, availableRoles []*libregraph.UnifiedRoleDefinition) (*libregraph.DriveItem, error) {
 
 	driveItem := libregraph.NewDriveItem()
 	permissions := make([]libregraph.Permission, 0, len(receivedShares))
@@ -347,7 +349,7 @@ func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.L
 			oldestReceivedShare = receivedShare
 		}
 
-		permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, receivedShare, resourceInfo)
+		permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, receivedShare, resourceInfo, availableRoles)
 		if err != nil {
 			return driveItem, err
 		}
@@ -408,7 +410,7 @@ func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.L
 
 func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Logger,
 	identityCache identity.IdentityCache, receivedShare *collaboration.ReceivedShare,
-	resourceInfo *storageprovider.ResourceInfo) (*libregraph.Permission, error) {
+	resourceInfo *storageprovider.ResourceInfo, availableRoles []*libregraph.UnifiedRoleDefinition) (*libregraph.Permission, error) {
 	permission := libregraph.NewPermission()
 	if id := receivedShare.GetShare().GetId().GetOpaqueId(); id != "" {
 		permission.SetId(id)
@@ -427,8 +429,12 @@ func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Lo
 		if err != nil {
 			return nil, err
 		}
-		role := unifiedrole.CS3ResourcePermissionsToDefinition(permissionSet, condition)
 
+		role := unifiedrole.CS3ResourcePermissionsToRole(
+			availableRoles,
+			permissionSet,
+			condition,
+		)
 		if role != nil {
 			permission.SetRoles([]string{role.GetId()})
 		}

services/graph/pkg/unifiedrole/conversion.go

@@ -142,17 +142,15 @@ func CS3ResourcePermissionsToLibregraphActions(p *provider.ResourcePermissions)
 	return actions
 }
 
-// CS3ResourcePermissionsToDefinition tries to find the UnifiedRoleDefinition that matches the supplied
+// CS3ResourcePermissionsToRole converts the provided cs3 ResourcePermissions to a libregraph UnifiedRoleDefinition
-// CS3 ResourcePermissions and constraints.
+func CS3ResourcePermissionsToRole(roleSet []*libregraph.UnifiedRoleDefinition, p *provider.ResourcePermissions, constraints string) *libregraph.UnifiedRoleDefinition {
-func _CS3ResourcePermissionsToDefinition(p *provider.ResourcePermissions, constraints string) *libregraph.UnifiedRoleDefinition {
-	a := CS3ResourcePermissionsToLibregraphActions(p)
 	actionSet := map[string]struct{}{}
-	for _, action := range a {
+	for _, action := range CS3ResourcePermissionsToLibregraphActions(p) {
 		actionSet[action] = struct{}{}
 	}
 
 	var res *libregraph.UnifiedRoleDefinition
-	for _, uRole := range GetDefinitions(RoleFilterAll()) {
+	for _, uRole := range roleSet {
 		matchFound := false
 		for _, uPerm := range uRole.GetRolePermissions() {
 			if uPerm.GetCondition() != constraints {
@@ -174,6 +172,7 @@ func _CS3ResourcePermissionsToDefinition(p *provider.ResourcePermissions, constr
 	return res
 }
 
+// resourceActionsEqual checks if the provided actions are equal to the actions defined for a resource
 func resourceActionsEqual(targetActionSet map[string]struct{}, actions []string) bool {
 	if len(targetActionSet) != len(actions) {
 		return false
@@ -187,16 +186,6 @@ func resourceActionsEqual(targetActionSet map[string]struct{}, actions []string)
 	return true
 }
 
-func CS3ResourcePermissionsToDefinition(p *provider.ResourcePermissions, constraints string) *libregraph.UnifiedRoleDefinition {
-	var res *libregraph.UnifiedRoleDefinition
-	matches := GetDefinitions(RoleFilterPermission(RoleFilterMatchExact, constraints, CS3ResourcePermissionsToLibregraphActions(p)...))
-	if len(matches) >= 1 {
-		res = matches[0]
-	}
-
-	return res
-}
-
 // cs3RoleToDisplayName converts a CS3 role to a human-readable display name
 func cs3RoleToDisplayName(role *conversions.Role) string {
 	if role == nil {

services/graph/pkg/unifiedrole/conversion_test.go

@@ -48,7 +48,7 @@ func TestPermissionsToCS3ResourcePermissions(t *testing.T) {
 	}
 }
 
-func TestCS3ResourcePermissionsToDefinition(t *testing.T) {
+func TestCS3ResourcePermissionsToRole(t *testing.T) {
 	tests := map[string]struct {
 		cs3ResourcePermissions *provider.ResourcePermissions
 		unifiedRoleDefinition  *libregraph.UnifiedRoleDefinition
@@ -69,7 +69,7 @@ func TestCS3ResourcePermissionsToDefinition(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			NewWithT(t).Expect(
-				unifiedrole.CS3ResourcePermissionsToDefinition(tc.cs3ResourcePermissions, tc.constraints),
+				unifiedrole.CS3ResourcePermissionsToRole(unifiedrole.BuildInRoles, tc.cs3ResourcePermissions, tc.constraints),
 			).To(Equal(tc.unifiedRoleDefinition))
 		})
 	}

services/graph/pkg/unifiedrole/errors.go

@@ -5,9 +5,6 @@ import (
 )
 
 var (
-	// ErrUnknownUnifiedRole is returned when an unknown unified role is requested.
+	// ErrUnknownRole is returned when an unknown unified role is requested.
-	ErrUnknownUnifiedRole = errors.New("unknown unified role, check if the role is enabled")
+	ErrUnknownRole = errors.New("unknown role, check if the role is enabled")
-
-	// ErrTooManyResults is returned when a filter returns too many results.
-	ErrTooManyResults = errors.New("too many results, consider using a more specific filter")
 )

services/graph/pkg/unifiedrole/export_test.go

@@ -1,7 +1,6 @@
 package unifiedrole
 
 var (
-	// roles
 	RoleViewer       = roleViewer
 	RoleSpaceViewer  = roleSpaceViewer
 	RoleEditor       = roleEditor
@@ -11,6 +10,7 @@ var (
 	RoleManager      = roleManager
 	RoleSecureViewer = roleSecureViewer
 
-	// functions
+	BuildInRoles = buildInRoles
-	WeightDefinitions = weightDefinitions
+
+	WeightDefinitions = weightRoles
 )

services/graph/pkg/unifiedrole/filter.go

@@ -8,18 +8,7 @@ import (
 
 type (
 	// RoleFilter is used to filter role collections
-	RoleFilter func(r *libregraph.UnifiedRoleDefinition) bool
+	RoleFilter func(*libregraph.UnifiedRoleDefinition) bool
-
-	// RoleFilterMatch defines the match behavior of a role filter
-	RoleFilterMatch int
-)
-
-const (
-	// RoleFilterMatchExact is the behavior for role filters that require an exact match
-	RoleFilterMatchExact RoleFilterMatch = iota
-
-	// RoleFilterMatchSome is the behavior for role filters that require some match
-	RoleFilterMatchSome
 )
 
 // RoleFilterInvert inverts the provided role filter
@@ -44,44 +33,12 @@ func RoleFilterIDs(ids ...string) RoleFilter {
 	}
 }
 
-// RoleFilterPermission returns a role filter that matches the provided condition and actions pair
-func RoleFilterPermission(matchBehavior RoleFilterMatch, condition string, wantActions ...string) RoleFilter {
-	return func(r *libregraph.UnifiedRoleDefinition) bool {
-		for _, permission := range r.GetRolePermissions() {
-			if permission.GetCondition() != condition {
-				continue
-			}
-
-			givenActions := permission.GetAllowedResourceActions()
-
-			switch {
-			case matchBehavior == RoleFilterMatchExact && slices.Equal(givenActions, wantActions):
-				return true
-			case matchBehavior == RoleFilterMatchSome:
-				matches := 0
-
-				for _, action := range givenActions {
-					if !slices.Contains(wantActions, action) {
-						break
-					}
-
-					matches++
-				}
-
-				return len(givenActions) == matches
-			}
-		}
-
-		return false
-	}
-}
-
 // filterRoles filters the provided roles by the provided filter
-func filterRoles(roles []*libregraph.UnifiedRoleDefinition, filter RoleFilter) []*libregraph.UnifiedRoleDefinition {
+func filterRoles(roles []*libregraph.UnifiedRoleDefinition, f RoleFilter) []*libregraph.UnifiedRoleDefinition {
 	return slices.DeleteFunc(
 		slices.Clone(roles),
 		func(r *libregraph.UnifiedRoleDefinition) bool {
-			return !filter(r)
+			return !f(r)
 		},
 	)
 }

services/graph/pkg/unifiedrole/filter_test.go

@@ -5,7 +5,6 @@ import (
 
 	. "github.com/onsi/gomega"
 	libregraph "github.com/owncloud/libre-graph-api-go"
-	"google.golang.org/protobuf/proto"
 
 	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 )
@@ -22,7 +21,9 @@ func TestRoleFilterIDs(t *testing.T) {
 func TestRoleFilterInvert(t *testing.T) {
 	NewWithT(t).Expect(
 		unifiedrole.RoleFilterInvert(
-			unifiedrole.RoleFilterAll(),
+			func(_ *libregraph.UnifiedRoleDefinition) bool {
+				return true
+			},
 		)(unifiedrole.RoleEditorLite),
 	).To(BeFalse())
 }
@@ -32,98 +33,3 @@ func TestRoleFilterAll(t *testing.T) {
 		unifiedrole.RoleFilterAll()(unifiedrole.RoleEditorLite),
 	).To(BeTrue())
 }
-
-func TestRoleFilterPermissions(t *testing.T) {
-	tests := map[string]struct {
-		unifiedRolePermission []libregraph.UnifiedRolePermission
-		filterCondition       string
-		filterActions         []string
-		filterMatch           bool
-	}{
-		"true | single": {
-			unifiedRolePermission: []libregraph.UnifiedRolePermission{
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionDrive),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemPermissionsCreate,
-					},
-				},
-			},
-			filterCondition: unifiedrole.UnifiedRoleConditionDrive,
-			filterActions: []string{
-				unifiedrole.DriveItemPermissionsCreate,
-			},
-			filterMatch: true,
-		},
-		"true | multiple": {
-			unifiedRolePermission: []libregraph.UnifiedRolePermission{
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionFolder),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemDeletedRead,
-					},
-				},
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionDrive),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemPermissionsCreate,
-					},
-				},
-			},
-			filterCondition: unifiedrole.UnifiedRoleConditionDrive,
-			filterActions: []string{
-				unifiedrole.DriveItemPermissionsCreate,
-			},
-			filterMatch: true,
-		},
-		"false | cross match": {
-			unifiedRolePermission: []libregraph.UnifiedRolePermission{
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionDrive),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemDeletedRead,
-					},
-				},
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionFolder),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemPermissionsCreate,
-					},
-				},
-			},
-			filterCondition: unifiedrole.UnifiedRoleConditionDrive,
-			filterActions:   []string{unifiedrole.DriveItemPermissionsCreate},
-			filterMatch:     false,
-		},
-		"false | too many actions": {
-			unifiedRolePermission: []libregraph.UnifiedRolePermission{
-				{
-					Condition: proto.String(unifiedrole.UnifiedRoleConditionDrive),
-					AllowedResourceActions: []string{
-						unifiedrole.DriveItemDeletedRead,
-						unifiedrole.DriveItemPermissionsCreate,
-					},
-				},
-			},
-			filterCondition: unifiedrole.UnifiedRoleConditionDrive,
-			filterActions: []string{
-				unifiedrole.DriveItemPermissionsCreate,
-			},
-			filterMatch: false,
-		},
-	}
-
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			NewWithT(t).Expect(
-				unifiedrole.RoleFilterPermission(
-					unifiedrole.RoleFilterMatchExact,
-					tc.filterCondition,
-					tc.filterActions...,
-				)(&libregraph.UnifiedRoleDefinition{
-					RolePermissions: tc.unifiedRolePermission,
-				}),
-			).To(Equal(tc.filterMatch))
-		})
-	}
-}

services/graph/pkg/unifiedrole/roles.go

@@ -278,41 +278,68 @@ var (
 	}()
 )
 
-// GetDefinitions returns a role filter that matches the provided resources
+// GetRoles returns a role filter that matches the provided resources
-func GetDefinitions(filter RoleFilter) []*libregraph.UnifiedRoleDefinition {
+func GetRoles(f RoleFilter) []*libregraph.UnifiedRoleDefinition {
-	return filterRoles(buildInRoles, filter)
+	return filterRoles(buildInRoles, f)
 }
 
-// GetDefinition returns a role filter that matches the provided resources
+// GetRole returns a role filter that matches the provided resources
-func GetDefinition(filter RoleFilter) (*libregraph.UnifiedRoleDefinition, error) {
+func GetRole(f RoleFilter) (*libregraph.UnifiedRoleDefinition, error) {
-	definitions := filterRoles(buildInRoles, filter)
+	roles := filterRoles(buildInRoles, f)
-	if len(definitions) == 0 {
+	if len(roles) == 0 {
-		return nil, ErrUnknownUnifiedRole
+		return nil, ErrUnknownRole
 	}
 
-	return definitions[0], nil
+	return roles[0], nil
 }
 
 // GetRolesByPermissions returns a list of role definitions
 // that match the provided actions and constraints
-func GetRolesByPermissions(actions []string, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
+func GetRolesByPermissions(roleSet []*libregraph.UnifiedRoleDefinition, actions []string, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
-	roles := GetDefinitions(RoleFilterPermission(RoleFilterMatchSome, constraints, actions...))
+	roles := make([]*libregraph.UnifiedRoleDefinition, 0, len(roleSet))
-	roles = weightDefinitions(roles, constraints, descending)
 
-	return roles
+	for _, role := range roleSet {
+		var match bool
+
+		for _, permission := range role.GetRolePermissions() {
+			if permission.GetCondition() != constraints {
+				continue
+			}
+
+			for i, action := range permission.GetAllowedResourceActions() {
+				if !slices.Contains(actions, action) {
+					break
+				}
+				if i == len(permission.GetAllowedResourceActions())-1 {
+					match = true
+				}
+			}
+
+			if match {
+				break
+			}
+		}
+
+		if match {
+			roles = append(roles, role)
+		}
+
+	}
+
+	return weightRoles(roles, constraints, descending)
 }
 
-// GetLegacyDefinitionName returns the legacy role name for the provided role
+// GetLegacyRoleName returns the legacy role name for the provided role
-func GetLegacyDefinitionName(definition libregraph.UnifiedRoleDefinition) string {
+func GetLegacyRoleName(role libregraph.UnifiedRoleDefinition) string {
-	return legacyNames[definition.GetId()]
+	return legacyNames[role.GetId()]
 }
 
-// weightDefinitions sorts the provided role definitions by the number of permissions[n].actions they grant,
+// weightRoles sorts the provided role definitions by the number of permissions[n].actions they grant,
 // the implementation is optimistic and assumes that the weight relies on the number of available actions.
 // descending - false - sorts the roles from least to most permissions
 // descending - true - sorts the roles from most to least permissions
-func weightDefinitions(definitions []*libregraph.UnifiedRoleDefinition, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
+func weightRoles(roleSet []*libregraph.UnifiedRoleDefinition, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
-	slices.SortFunc(definitions, func(i, j *libregraph.UnifiedRoleDefinition) int {
+	slices.SortFunc(roleSet, func(i, j *libregraph.UnifiedRoleDefinition) int {
 		var ia []string
 		for _, rp := range i.GetRolePermissions() {
 			if rp.GetCondition() == constraints {
@@ -335,12 +362,12 @@ func weightDefinitions(definitions []*libregraph.UnifiedRoleDefinition, constrai
 		}
 	})
 
-	for i, definition := range definitions {
+	for i, role := range roleSet {
-		definition.LibreGraphWeight = libregraph.PtrInt32(int32(i) + 1)
+		role.LibreGraphWeight = libregraph.PtrInt32(int32(i) + 1)
 	}
 
 	// return for the sake of consistency, optional because the slice is modified in place
-	return definitions
+	return roleSet
 }
 
 // GetAllowedResourceActions returns the allowed resource actions for the provided role by condition

services/graph/pkg/unifiedrole/roles_test.go

@@ -27,14 +27,14 @@ func TestGetDefinition(t *testing.T) {
 		},
 		"fail unknown": {
 			ids:         []string{"unknown"},
-			expectError: unifiedrole.ErrUnknownUnifiedRole,
+			expectError: unifiedrole.ErrUnknownRole,
 		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			g := NewWithT(t)
-			definition, err := unifiedrole.GetDefinition(unifiedrole.RoleFilterIDs(tc.ids...))
+			definition, err := unifiedrole.GetRole(unifiedrole.RoleFilterIDs(tc.ids...))
 
 			if tc.expectError != nil {
 				g.Expect(err).To(MatchError(tc.expectError))
@@ -96,7 +96,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 		unifiedRoleDefinition []*libregraph.UnifiedRoleDefinition
 	}{
 		"ViewerUnifiedRole": {
-			givenActions: rolesToAction(unifiedrole.RoleViewer),
+			givenActions: getRoleActions(unifiedrole.RoleViewer),
 			constraints:  unifiedrole.UnifiedRoleConditionFolder,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -104,7 +104,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"ViewerUnifiedRole | share": {
-			givenActions: rolesToAction(unifiedrole.RoleViewer),
+			givenActions: getRoleActions(unifiedrole.RoleViewer),
 			constraints:  unifiedrole.UnifiedRoleConditionFile,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -112,7 +112,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"NewFileEditorUnifiedRole": {
-			givenActions: rolesToAction(unifiedrole.RoleFileEditor),
+			givenActions: getRoleActions(unifiedrole.RoleFileEditor),
 			constraints:  unifiedrole.UnifiedRoleConditionFile,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -121,7 +121,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"NewEditorUnifiedRole": {
-			givenActions: rolesToAction(unifiedrole.RoleEditor),
+			givenActions: getRoleActions(unifiedrole.RoleEditor),
 			constraints:  unifiedrole.UnifiedRoleConditionFolder,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -131,7 +131,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"GetRoles 1": {
-			givenActions: rolesToAction(unifiedrole.GetDefinitions(unifiedrole.RoleFilterAll())...),
+			givenActions: getRoleActions(unifiedrole.BuildInRoles...),
 			constraints:  unifiedrole.UnifiedRoleConditionFile,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -140,7 +140,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"GetRoles 2": {
-			givenActions: rolesToAction(unifiedrole.GetDefinitions(unifiedrole.RoleFilterAll())...),
+			givenActions: getRoleActions(unifiedrole.BuildInRoles...),
 			constraints:  unifiedrole.UnifiedRoleConditionFolder,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -150,7 +150,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			},
 		},
 		"GetRoles 3": {
-			givenActions: rolesToAction(unifiedrole.GetDefinitions(unifiedrole.RoleFilterAll())...),
+			givenActions: getRoleActions(unifiedrole.BuildInRoles...),
 			constraints:  unifiedrole.UnifiedRoleConditionDrive,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSpaceViewer,
@@ -164,7 +164,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{},
 		},
 		"mixed": {
-			givenActions: append(rolesToAction(unifiedrole.RoleEditorLite), unifiedrole.DriveItemQuotaRead),
+			givenActions: append(getRoleActions(unifiedrole.RoleEditorLite), unifiedrole.DriveItemQuotaRead),
 			constraints:  unifiedrole.UnifiedRoleConditionFolder,
 			unifiedRoleDefinition: []*libregraph.UnifiedRoleDefinition{
 				unifiedrole.RoleSecureViewer,
@@ -176,7 +176,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			g := NewWithT(t)
-			generatedDefinitions := unifiedrole.GetRolesByPermissions(tc.givenActions, tc.constraints, false)
+			generatedDefinitions := unifiedrole.GetRolesByPermissions(unifiedrole.BuildInRoles, tc.givenActions, tc.constraints, false)
 
 			g.Expect(len(generatedDefinitions)).To(Equal(len(tc.unifiedRoleDefinition)))
 
@@ -185,7 +185,7 @@ func TestGetRolesByPermissions(t *testing.T) {
 				g.Expect(*generatedDefinition.LibreGraphWeight).To(Equal(int32(i + 1)))
 			}
 
-			generatedActions := rolesToAction(generatedDefinitions...)
+			generatedActions := getRoleActions(generatedDefinitions...)
 
 			g.Expect(len(tc.givenActions) >= len(generatedActions)).To(BeTrue())
 			for _, generatedAction := range generatedActions {

services/graph/pkg/unifiedrole/unifiedrole_suite_test.go

@@ -6,7 +6,7 @@ import (
 	libregraph "github.com/owncloud/libre-graph-api-go"
 )
 
-func rolesToAction(definitions ...*libregraph.UnifiedRoleDefinition) []string {
+func getRoleActions(definitions ...*libregraph.UnifiedRoleDefinition) []string {
 	var actions []string
 
 	for _, definition := range definitions {

services/graph/pkg/validate/libregraph.go

@@ -90,9 +90,10 @@ func rolesAndActions(ctx context.Context, sl validator.StructLevel, roles, actio
 
 	switch roles, ok := ctx.Value(_contextRoleIDsValueKey).([]string); {
 	case ok:
-		definitions = unifiedrole.GetDefinitions(unifiedrole.RoleFilterIDs(roles...))
+		definitions = unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(roles...))
 	default:
-		definitions = unifiedrole.GetDefinitions(unifiedrole.RoleFilterAll())
+		// it the ctx does not contain the allowed role IDs, we need to fall back to all roles
+		definitions = unifiedrole.GetRoles(unifiedrole.RoleFilterAll())
 	}
 
 	for _, definition := range definitions {
@@ -121,8 +122,8 @@ func rolesAndActions(ctx context.Context, sl validator.StructLevel, roles, actio
 		sl.ReportError(roles, "Roles", "Roles", "available_role", "")
 	}
 
-	for _, role := range actions {
+	for _, action := range actions {
-		if slices.Contains(availableActions, role) {
+		if slices.Contains(availableActions, action) {
 			continue
 		}
 

services/graph/pkg/validate/libregraph_test.go

@@ -16,10 +16,12 @@ import (
 type validatableFactory[T any] func() (T, bool)
 
 var _ = Describe("libregraph", func() {
+	var ctx context.Context
 	var driveItemInvite libregraph.DriveItemInvite
 	var driveRecipient libregraph.DriveRecipient
 
 	BeforeEach(func() {
+		ctx = context.Background()
 		driveRecipient = libregraph.DriveRecipient{
 			ObjectId:                conversions.ToPointer("1"),
 			LibreGraphRecipientType: conversions.ToPointer("user"),
@@ -27,18 +29,17 @@ var _ = Describe("libregraph", func() {
 
 		driveItemInvite = libregraph.DriveItemInvite{
 			Recipients:                   []libregraph.DriveRecipient{driveRecipient},
-			Roles:                        []string{role.UnifiedRoleEditorID},
+			Roles:                        []string{unifiedrole.UnifiedRoleEditorID},
-			LibreGraphPermissionsActions: []string{role.DriveItemVersionsUpdate},
+			LibreGraphPermissionsActions: []string{unifiedrole.DriveItemVersionsUpdate},
 			ExpirationDateTime:           libregraph.PtrTime(time.Now().Add(time.Hour)),
 		}
-
 	})
 
 	DescribeTable("DriveItemInvite",
 		func(factories ...validatableFactory[libregraph.DriveItemInvite]) {
 			for _, factory := range factories {
 				s, pass := factory()
-				switch err := validate.StructCtx(context.Background(), s); pass {
+				switch err := validate.StructCtx(ctx, s); pass {
 				case false:
 					Expect(err).To(HaveOccurred())
 				default:
@@ -61,8 +62,8 @@ var _ = Describe("libregraph", func() {
 		}),
 		Entry("fail: multiple role assignment", func() (libregraph.DriveItemInvite, bool) {
 			driveItemInvite.Roles = []string{
-				role.UnifiedRoleEditorID,
+				unifiedrole.UnifiedRoleEditorID,
-				role.UnifiedRoleManagerID,
+				unifiedrole.UnifiedRoleManagerID,
 			}
 			driveItemInvite.LibreGraphPermissionsActions = nil
 			return driveItemInvite, false
@@ -72,6 +73,14 @@ var _ = Describe("libregraph", func() {
 			driveItemInvite.LibreGraphPermissionsActions = nil
 			return driveItemInvite, false
 		}),
+		Entry("fail: disabled role", func() (libregraph.DriveItemInvite, bool) {
+			ctx = validate.ContextWithAllowedRoleIDs(ctx, []string{unifiedrole.UnifiedRoleEditorID})
+			driveItemInvite.Roles = []string{
+				unifiedrole.UnifiedRoleSecureViewerID,
+			}
+			driveItemInvite.LibreGraphPermissionsActions = nil
+			return driveItemInvite, false
+		}),
 		Entry("fail: unknown action", func() (libregraph.DriveItemInvite, bool) {
 			driveItemInvite.Roles = nil
 			driveItemInvite.LibreGraphPermissionsActions = []string{"foo"}
@@ -84,8 +93,8 @@ var _ = Describe("libregraph", func() {
 		}),
 		Entry("fail: different number of roles and actions", func() (libregraph.DriveItemInvite, bool) {
 			driveItemInvite.LibreGraphPermissionsActions = []string{
-				role.DriveItemVersionsUpdate,
+				unifiedrole.DriveItemVersionsUpdate,
-				role.DriveItemChildrenCreate,
+				unifiedrole.DriveItemChildrenCreate,
 			}
 			return driveItemInvite, false
 		}),

services/web/pkg/theme/service_test.go

@@ -70,5 +70,5 @@ func TestService_Get(t *testing.T) {
 	// brandingTheme
 	assert.Equal(t, jsonData.Get("_branding").String(), "_branding")
 	// themeDefaults
-	assert.Equal(t, jsonData.Get("common.shareRoles."+role.UnifiedRoleViewerID+".name").String(), "UnifiedRoleViewer")
+	assert.Equal(t, jsonData.Get("common.shareRoles."+unifiedrole.UnifiedRoleViewerID+".name").String(), "UnifiedRoleViewer")
 }