enhancement: do not enable all roles by default.

from now on, not all unified roles are enabled by default, instead the available roles are hand-picked in the default setup.

For advanced use-cases, the administrator is capable to enable the desired set of available roles.

Picking roles is not easy since the uid is NOT humanly readable, therefore a cli is contained which lists the available, disabled and enabled roles.

services/graph/pkg/command/root.go

@@ -5,13 +5,14 @@ import (
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/clihelper"
 
-	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
 	"github.com/urfave/cli/v2"
+
+	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
 )
 
 // GetCommands provides all commands for this service
 func GetCommands(cfg *config.Config) cli.Commands {
-	return []*cli.Command{
+	return append([]*cli.Command{
 		// start this service
 		Server(cfg),
 
@@ -20,7 +21,7 @@ func GetCommands(cfg *config.Config) cli.Commands {
 		// infos about this service
 		Health(cfg),
 		Version(cfg),
-	}
+	}, UnifiedRoles(cfg)...)
 }
 
 // Execute is the entry point for the ocis-graph command.

services/graph/pkg/command/unified_roles.go

@@ -0,0 +1,80 @@
+package command
+
+import (
+	"fmt"
+	"os"
+	"slices"
+	"strings"
+
+	"github.com/charmbracelet/lipgloss"
+	"github.com/charmbracelet/lipgloss/table"
+	"github.com/urfave/cli/v2"
+
+	"github.com/owncloud/ocis/v2/ocis-pkg/config/configlog"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/config/parser"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
+)
+
+// UnifiedRoles bundles available commands for unified roles
+func UnifiedRoles(cfg *config.Config) cli.Commands {
+	cmds := cli.Commands{
+		unifiedRolesStatus(cfg),
+	}
+
+	for _, cmd := range cmds {
+		cmd.Category = "unified-roles"
+		cmd.Name = strings.Join([]string{cmd.Name, "unified-roles"}, "-")
+		cmd.Before = func(c *cli.Context) error {
+			return configlog.ReturnError(parser.ParseConfig(cfg))
+		}
+	}
+
+	return cmds
+}
+
+// unifiedRolesStatus lists available unified roles, it contains an indicator to show if the role is enabled or not
+func unifiedRolesStatus(cfg *config.Config) *cli.Command {
+	return &cli.Command{
+		Name:  "list",
+		Usage: "list available unified roles",
+		Action: func(c *cli.Context) error {
+			re := lipgloss.NewRenderer(os.Stdout)
+			baseStyle := re.NewStyle().Padding(0, 1)
+
+			var data [][]string
+
+			for _, definition := range unifiedrole.GetBuiltinRoleDefinitionList() {
+				data = append(data, []string{"", definition.GetId(), definition.GetDescription()})
+			}
+
+			t := table.New().
+				Border(lipgloss.NormalBorder()).
+				Headers("Enabled", "UID", "Description").
+				Rows(data...).
+				StyleFunc(func(row, col int) lipgloss.Style {
+					if row == 0 {
+						return baseStyle.Foreground(lipgloss.Color("252")).Bold(true)
+					}
+
+					if row != 0 && col == 0 {
+						indicatorStyle := baseStyle.Align(lipgloss.Center)
+
+						// Check if the role is enabled, header takes up the first row
+						switch slices.Contains(cfg.UnifiedRoles.AvailableRoles, data[row-1][1]) {
+						case true:
+							return indicatorStyle.Background(lipgloss.Color("34")) // ANSI green
+						default:
+							return indicatorStyle.Background(lipgloss.Color("9")) // ANSI red
+						}
+					}
+
+					return baseStyle
+				})
+
+			fmt.Println(t)
+
+			return nil
+		},
+	}
+}

services/graph/pkg/config/config.go

@@ -30,6 +30,7 @@ type Config struct {
 	Identity          Identity    `yaml:"identity"`
 	IncludeOCMSharees bool        `yaml:"include_ocm_sharees" env:"OCIS_ENABLE_OCM;GRAPH_INCLUDE_OCM_SHAREES" desc:"Include OCM sharees when listing users." introductionVersion:"5.0"`
 	Events            Events      `yaml:"events"`
+	UnifiedRoles      UnifiedRoles `yaml:"unified_roles"`
 
 	Keycloak       Keycloak       `yaml:"keycloak"`
 	ServiceAccount ServiceAccount `yaml:"service_account"`

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

@@ -9,6 +9,13 @@ import (
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
 	"github.com/owncloud/ocis/v2/ocis-pkg/structs"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
+)
+
+var (
+	// _disabledByDefaultUnifiedRoleRoleIDs contains all roles that are not enabled by default,
+	// but can be enabled by the user.
+	_disabledByDefaultUnifiedRoleRoleIDs = []string{unifiedrole.UnifiedRoleSecureViewerID}
 )
 
 // FullDefaultConfig returns a fully initialized default configuration
@@ -164,6 +171,16 @@ func EnsureDefaults(cfg *config.Config) {
 	if cfg.Identity.LDAP.GroupCreateBaseDN == "" {
 		cfg.Identity.LDAP.GroupCreateBaseDN = cfg.Identity.LDAP.GroupBaseDN
 	}
+
+	// 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.GetBuiltinRoleDefinitionList(
+			// filter out the roles that are disabled by default
+			unifiedrole.RoleFilterInvert(unifiedrole.RoleFilterIDs(_disabledByDefaultUnifiedRoleRoleIDs...)),
+		) {
+			cfg.UnifiedRoles.AvailableRoles = append(cfg.UnifiedRoles.AvailableRoles, definition.GetId())
+		}
+	}
 }
 
 // Sanitize sanitized the configuration

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

@@ -5,11 +5,13 @@ import (
 	"fmt"
 
 	"github.com/go-ldap/ldap/v3"
+
 	ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config"
 	defaults2 "github.com/owncloud/ocis/v2/ocis-pkg/config/defaults"
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults"
+	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/config/envdecode"
 )
@@ -72,6 +74,23 @@ func Validate(cfg *config.Config) error {
 		return shared.MissingServiceAccountSecret(cfg.Service.Name)
 	}
 
+	// validate unified roles
+	{
+		var err error
+
+		for _, uid := range cfg.UnifiedRoles.AvailableRoles {
+			// check if the role is known
+			if len(unifiedrole.GetBuiltinRoleDefinitionList(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))
+			}
+		}
+
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

services/graph/pkg/config/unified_roles.go

@@ -0,0 +1,6 @@
+package config
+
+// UnifiedRoles contains all settings related to unified roles.
+type UnifiedRoles struct {
+	AvailableRoles []string `yaml:"available_roles" env:"UNIFIED_ROLES_AVAILABLE_ROLES" desc:"A list of roles that are available for assignment." introductionVersion:"%%NEXT%%"`
+}

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

@@ -8,6 +8,7 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/render"
 	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"
 )
@@ -15,6 +16,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)
+	// fixMe: should we consider all roles or only the ones that are enabled?
 	render.JSON(w, r, unifiedrole.GetBuiltinRoleDefinitionList())
 }
 
@@ -38,6 +40,7 @@ func (g Graph) GetRoleDefinition(w http.ResponseWriter, r *http.Request) {
 }
 
 func getRoleDefinition(roleID string) (*libregraph.UnifiedRoleDefinition, error) {
+	// fixMe: should we consider all roles or only the ones that are enabled?
 	roleList := unifiedrole.GetBuiltinRoleDefinitionList()
 	for _, role := range roleList {
 		if role != nil && role.Id != nil && *role.Id == roleID {

services/graph/pkg/unifiedrole/errors.go

@@ -0,0 +1,10 @@
+package unifiedrole
+
+import (
+	"errors"
+)
+
+var (
+	// ErrUnknownUnifiedRole is returned when an unknown unified role is requested.
+	ErrUnknownUnifiedRole = errors.New("unknown unified role")
+)

services/graph/pkg/unifiedrole/unifiedrole.go

@@ -4,7 +4,6 @@ import (
 	"cmp"
 	"errors"
 	"slices"
-	"strings"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	libregraph "github.com/owncloud/libre-graph-api-go"
@@ -14,6 +13,31 @@ import (
 	"github.com/cs3org/reva/v2/pkg/conversions"
 )
 
+// roleFilter is used to filter role collections
+type roleFilter func(r *libregraph.UnifiedRoleDefinition) bool
+
+var (
+	// RoleFilterInvert inverts the provided role filter
+	RoleFilterInvert = func(f roleFilter) roleFilter {
+		return func(r *libregraph.UnifiedRoleDefinition) bool {
+			return !f(r)
+		}
+	}
+
+	// RoleFilterIDs returns a role filter that matches the provided ids
+	RoleFilterIDs = func(ids ...string) roleFilter {
+		return func(r *libregraph.UnifiedRoleDefinition) bool {
+			for _, id := range ids {
+				if r.GetId() == id {
+					return true
+				}
+			}
+
+			return false
+		}
+	}
+)
+
 const (
 	// UnifiedRoleViewerID Unified role viewer id.
 	UnifiedRoleViewerID = "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
@@ -31,18 +55,6 @@ 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. For 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"
@@ -51,19 +63,6 @@ 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"
@@ -85,7 +84,7 @@ const (
 	DriveItemPermissionsDeny   = "libre.graph/driveItem/permissions/deny"
 )
 
-var legacyNames map[string]string = map[string]string{
+var legacyNames = map[string]string{
 	UnifiedRoleViewerID: conversions.RoleViewer,
 	// one V1 api the "spaceviewer" role was call "viewer" and the "spaceeditor" was "editor",
 	// we need to stay compatible with that
@@ -156,14 +155,6 @@ func NewViewerUnifiedRole() *libregraph.UnifiedRoleDefinition {
 				AllowedResourceActions: convert(r),
 				Condition:              proto.String(UnifiedRoleConditionFolder),
 			},
-			{
-				AllowedResourceActions: convert(r),
-				Condition:              proto.String(UnifiedRoleConditionFileFederatedUser),
-			},
-			{
-				AllowedResourceActions: convert(r),
-				Condition:              proto.String(UnifiedRoleConditionFolderFederatedUser),
-			},
 		},
 		LibreGraphWeight: proto.Int32(0),
 	}
@@ -198,10 +189,6 @@ func NewEditorUnifiedRole() *libregraph.UnifiedRoleDefinition {
 				AllowedResourceActions: convert(r),
 				Condition:              proto.String(UnifiedRoleConditionFolder),
 			},
-			{
-				AllowedResourceActions: convert(r),
-				Condition:              proto.String(UnifiedRoleConditionFolderFederatedUser),
-			},
 		},
 		LibreGraphWeight: proto.Int32(0),
 	}
@@ -236,10 +223,6 @@ func NewFileEditorUnifiedRole() *libregraph.UnifiedRoleDefinition {
 				AllowedResourceActions: convert(r),
 				Condition:              proto.String(UnifiedRoleConditionFile),
 			},
-			{
-				AllowedResourceActions: convert(r),
-				Condition:              proto.String(UnifiedRoleConditionFileFederatedUser),
-			},
 		},
 		LibreGraphWeight: proto.Int32(0),
 	}
@@ -302,6 +285,7 @@ func NewSecureViewerUnifiedRole() *libregraph.UnifiedRoleDefinition {
 
 // NewUnifiedRoleFromID returns a unified role definition from the provided id
 func NewUnifiedRoleFromID(id string) (*libregraph.UnifiedRoleDefinition, error) {
+	// fixMe: should we consider all roles or only the ones that are enabled?
 	for _, definition := range GetBuiltinRoleDefinitionList() {
 		if definition.GetId() != id {
 			continue
@@ -313,8 +297,8 @@ func NewUnifiedRoleFromID(id string) (*libregraph.UnifiedRoleDefinition, error)
 	return nil, errors.New("role not found")
 }
 
-func GetBuiltinRoleDefinitionList() []*libregraph.UnifiedRoleDefinition {
-	return []*libregraph.UnifiedRoleDefinition{
+func GetBuiltinRoleDefinitionList(filter ...roleFilter) []*libregraph.UnifiedRoleDefinition {
+	roles := []*libregraph.UnifiedRoleDefinition{
 		NewViewerUnifiedRole(),
 		NewSpaceViewerUnifiedRole(),
 		NewEditorUnifiedRole(),
@@ -324,11 +308,20 @@ func GetBuiltinRoleDefinitionList() []*libregraph.UnifiedRoleDefinition {
 		NewManagerUnifiedRole(),
 		NewSecureViewerUnifiedRole(),
 	}
+
+	for _, f := range filter {
+		roles = slices.DeleteFunc(roles, func(r *libregraph.UnifiedRoleDefinition) bool {
+			return !f(r)
+		})
+	}
+
+	return roles
 }
 
 // GetApplicableRoleDefinitionsForActions returns a list of role definitions
 // that match the provided actions and constraints
-func GetApplicableRoleDefinitionsForActions(actions []string, constraints string, listFederatedRoles, descending bool) []*libregraph.UnifiedRoleDefinition {
+func GetApplicableRoleDefinitionsForActions(actions []string, constraints string, descending bool) []*libregraph.UnifiedRoleDefinition {
+	// fixMe: should we consider all roles or only the ones that are enabled?
 	builtin := GetBuiltinRoleDefinitionList()
 	definitions := make([]*libregraph.UnifiedRoleDefinition, 0, len(builtin))
 
@@ -336,14 +329,7 @@ func GetApplicableRoleDefinitionsForActions(actions []string, constraints string
 		var definitionMatch bool
 
 		for _, permission := range definition.GetRolePermissions() {
-			// 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:
+			if permission.GetCondition() != constraints {
 				continue
 			}
 
@@ -534,6 +520,7 @@ func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constr
 	}
 
 	var res *libregraph.UnifiedRoleDefinition
+	// fixMe: should we consider all roles or only the ones that are enabled?
 	for _, uRole := range GetBuiltinRoleDefinitionList() {
 		matchFound := false
 		for _, uPerm := range uRole.GetRolePermissions() {

services/graph/pkg/validate/libregraph.go

@@ -84,6 +84,8 @@ func rolesAndActions(sl validator.StructLevel, roles, actions []string, allowEmp
 	var availableRoles []string
 	var availableActions []string
 	for _, definition := range append(
+		// fixMe: why twice!?
+		// fixMe: should we consider all roles or only the ones that are enabled?
 		unifiedrole.GetBuiltinRoleDefinitionList(),
 		unifiedrole.GetBuiltinRoleDefinitionList()...,
 	) {