enhancement: add allowed role validation to the go-playground validator

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

@@ -114,6 +114,9 @@ func DefaultConfig() *config.Config {
 			Cluster:   "ocis-cluster",
 			EnableTLS: false,
 		},
+		UnifiedRoles: config.UnifiedRoles{
+			AvailableRoles: nil, // will be populated with defaults in EnsureDefaults
+		},
 	}
 }
 

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

@@ -73,13 +73,13 @@ const (
 )
 
 // NewDriveItemPermissionsService creates a new DriveItemPermissionsService
-func NewDriveItemPermissionsService(logger log.Logger, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], identityCache identity.IdentityCache, config *config.Config) (DriveItemPermissionsService, error) {
+func NewDriveItemPermissionsService(logger log.Logger, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], identityCache identity.IdentityCache, c *config.Config) (DriveItemPermissionsService, error) {
 	return DriveItemPermissionsService{
 		BaseGraphService: BaseGraphService{
 			logger:          &log.Logger{Logger: logger.With().Str("graph api", "DrivesDriveItemService").Logger()},
 			gatewaySelector: gatewaySelector,
 			identityCache:   identityCache,
-			config:          config,
+			config:          c,
 		},
 	}, nil
 }
@@ -104,6 +104,11 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 
 	unifiedRolePermissions := []*libregraph.UnifiedRolePermission{{AllowedResourceActions: invite.LibreGraphPermissionsActions}}
 	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
+		}
+
 		role, err := unifiedrole.NewUnifiedRoleFromID(roleID)
 		if err != nil {
 			s.logger.Debug().Err(err).Interface("role", invite.GetRoles()[0]).Msg("unable to convert requested role")
@@ -544,18 +549,20 @@ func (s DriveItemPermissionsService) UpdateSpaceRootPermission(ctx context.Conte
 	return s.UpdatePermission(ctx, rootResourceID, permissionID, newPermission)
 }
 
-// DriveItemPermissionsService is the api that registers the http endpoints which expose needed operation to the graph api.
+// DriveItemPermissionsApi is the api that registers the http endpoints which expose needed operation to the graph api.
 // the business logic is delegated to the permissions service and further down to the cs3 client.
 type DriveItemPermissionsApi struct {
 	logger                      log.Logger
 	driveItemPermissionsService DriveItemPermissionsProvider
+	config                      *config.Config
 }
 
 // NewDriveItemPermissionsApi creates a new DriveItemPermissionsApi
-func NewDriveItemPermissionsApi(driveItemPermissionService DriveItemPermissionsProvider, logger log.Logger) (DriveItemPermissionsApi, error) {
+func NewDriveItemPermissionsApi(driveItemPermissionService DriveItemPermissionsProvider, logger log.Logger, c *config.Config) (DriveItemPermissionsApi, error) {
 	return DriveItemPermissionsApi{
 		logger:                      log.Logger{Logger: logger.With().Str("graph api", "DrivesDriveItemApi").Logger()},
 		driveItemPermissionsService: driveItemPermissionService,
+		config:                      c,
 	}, nil
 }
 
@@ -575,15 +582,14 @@ func (api DriveItemPermissionsApi) Invite(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	ctx := r.Context()
+	ctx := validate.ContextWithAllowedRoleIDs(r.Context(), api.config.UnifiedRoles.AvailableRoles)
-
 	if err = validate.StructCtx(ctx, driveItemInvite); err != nil {
 		api.logger.Debug().Err(err).Interface("Body", r.Body).Msg("invalid request body")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
 		return
 	}
-	permission, err := api.driveItemPermissionsService.Invite(ctx, &itemID, *driveItemInvite)
 
+	permission, err := api.driveItemPermissionsService.Invite(ctx, &itemID, *driveItemInvite)
 	if err != nil {
 		errorcode.RenderError(w, r, err)
 		return
@@ -609,7 +615,7 @@ func (api DriveItemPermissionsApi) SpaceRootInvite(w http.ResponseWriter, r *htt
 		return
 	}
 
-	ctx := r.Context()
+	ctx := validate.ContextWithAllowedRoleIDs(r.Context(), api.config.UnifiedRoles.AvailableRoles)
 	if err = validate.StructCtx(ctx, driveItemInvite); err != nil {
 		api.logger.Debug().Err(err).Interface("Body", r.Body).Msg("invalid request body")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())

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

@@ -245,6 +245,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(permission.GetLibreGraphPermissionsActions()).To(HaveLen(1))
 			Expect(permission.GetLibreGraphPermissionsActions()[0]).To(Equal(unifiedrole.DriveItemContentRead))
 		})
+
 		It("fails with a missing driveritem", func() {
 			statResponse.Status = status.NewNotFound(context.Background(), "not found")
 			permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
@@ -252,6 +253,12 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(err).To(MatchError(errorcode.New(errorcode.ItemNotFound, "not found").WithOrigin(errorcode.ErrorOriginCS3)))
 			Expect(permission).To(BeZero())
 		})
+
+		It("fails with unknown or disable role", func() {
+			driveItemInvite.Roles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleSecureViewerID}
+			_, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
+			Expect(err).To(MatchError(unifiedrole.ErrUnknownUnifiedRole))
+		})
 	})
 	Describe("SpaceRootInvite", func() {
 		var (
@@ -1015,7 +1022,7 @@ var _ = Describe("DriveItemPermissionsApi", func() {
 		logger := log.NewLogger()
 
 		mockProvider = mocks.NewDriveItemPermissionsProvider(GinkgoT())
-		api, err := svc.NewDriveItemPermissionsApi(mockProvider, logger)
+		api, err := svc.NewDriveItemPermissionsApi(mockProvider, logger, defaults.FullDefaultConfig())
 		Expect(err).ToNot(HaveOccurred())
 
 		httpAPI = api
@@ -1097,6 +1104,22 @@ var _ = Describe("DriveItemPermissionsApi", func() {
 
 			Expect(responseRecorder.Code).To(Equal(http.StatusOK))
 		})
+
+		It("fails with unknown or disable role", func() {
+			rCTX.URLParams.Add("itemID", "1$2!3")
+			responseRecorder := httptest.NewRecorder()
+			invite.Roles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleSecureViewerID}
+			inviteJson, err := json.Marshal(invite)
+			Expect(err).ToNot(HaveOccurred())
+
+			request := httptest.NewRequest(http.MethodPost, "/", bytes.NewBuffer(inviteJson)).
+				WithContext(
+					context.WithValue(context.Background(), chi.RouteCtxKey, rCTX),
+				)
+			httpAPI.Invite(responseRecorder, request)
+
+			Expect(responseRecorder.Code).To(Equal(http.StatusBadRequest))
+		})
 	})
 	Describe("SpaceRootInvite", func() {
 		It("call the Invite provider with the correct arguments", func() {
@@ -1132,6 +1155,21 @@ var _ = Describe("DriveItemPermissionsApi", func() {
 
 			Expect(responseRecorder.Code).To(Equal(http.StatusUnprocessableEntity))
 		})
+
+		It("fails with unknown or disable role", func() {
+			responseRecorder := httptest.NewRecorder()
+			invite.Roles = []string{unifiedrole.UnifiedRoleViewerID, unifiedrole.UnifiedRoleSecureViewerID}
+			inviteJson, err := json.Marshal(invite)
+			Expect(err).ToNot(HaveOccurred())
+
+			request := httptest.NewRequest(http.MethodPost, "/", bytes.NewBuffer(inviteJson)).
+				WithContext(
+					context.WithValue(context.Background(), chi.RouteCtxKey, rCTX),
+				)
+			httpAPI.SpaceRootInvite(responseRecorder, request)
+
+			Expect(responseRecorder.Code).To(Equal(http.StatusBadRequest))
+		})
 	})
 	Describe("ListPermissions", func() {
 		It("calls the ListPermissions provider with the correct arguments", func() {

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

@@ -1,7 +1,6 @@
 package svc
 
 import (
-	"fmt"
 	"net/http"
 	"net/url"
 
@@ -16,8 +15,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(unifiedrole.RoleFilterIDs(g.config.UnifiedRoles.AvailableRoles...)))
-	render.JSON(w, r, unifiedrole.GetBuiltinRoleDefinitionList())
 }
 
 // GetRoleDefinition a permission role than can be used when sharing with users or groups
@@ -29,7 +27,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 := getRoleDefinition(roleID)
+	role, err := getRoleDefinition(roleID, g.config.UnifiedRoles.AvailableRoles)
 	if err != nil {
 		logger.Debug().Str("roleID", roleID).Msg("could not get role: not found")
 		errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, err.Error())
@@ -39,13 +37,11 @@ func (g Graph) GetRoleDefinition(w http.ResponseWriter, r *http.Request) {
 	render.JSON(w, r, role)
 }
 
-func getRoleDefinition(roleID string) (*libregraph.UnifiedRoleDefinition, error) {
+func getRoleDefinition(roleID string, availableRoles []string) (*libregraph.UnifiedRoleDefinition, error) {
-	// fixMe: should we consider all roles or only the ones that are enabled?
+	for _, role := range unifiedrole.GetBuiltinRoleDefinitionList(unifiedrole.RoleFilterIDs(availableRoles...)) {
-	roleList := unifiedrole.GetBuiltinRoleDefinitionList()
-	for _, role := range roleList {
 		if role != nil && role.Id != nil && *role.Id == roleID {
 			return role, nil
 		}
 	}
-	return nil, fmt.Errorf("role not found")
+	return nil, unifiedrole.ErrUnknownUnifiedRole
 }

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

@@ -214,7 +214,7 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
 		return svc, err
 	}
 
-	driveItemPermissionsApi, err := NewDriveItemPermissionsApi(driveItemPermissionsService, options.Logger)
+	driveItemPermissionsApi, err := NewDriveItemPermissionsApi(driveItemPermissionsService, options.Logger, options.Config)
 	if err != nil {
 		return svc, err
 	}

services/graph/pkg/unifiedrole/errors.go

@@ -6,5 +6,5 @@ import (
 
 var (
 	// ErrUnknownUnifiedRole is returned when an unknown unified role is requested.
-	ErrUnknownUnifiedRole = errors.New("unknown unified role")
+	ErrUnknownUnifiedRole = errors.New("unknown unified role, check if the role is enabled")
 )

services/graph/pkg/validate/libregraph.go

@@ -1,6 +1,7 @@
 package validate
 
 import (
+	"context"
 	"slices"
 
 	"github.com/go-playground/validator/v10"
@@ -9,6 +10,10 @@ import (
 	"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
 )
 
+var (
+	_contextRoleIDsValueKey = "roleFilterIDs"
+)
+
 // initLibregraph initializes libregraph validation
 func initLibregraph(v *validator.Validate) {
 	for _, f := range []func(*validator.Validate){
@@ -30,10 +35,9 @@ func libregraphDriveItemInvite(v *validator.Validate) {
 		"ExpirationDateTime": "omitnil,gt",
 	}, s)
 
-	v.RegisterStructValidation(func(sl validator.StructLevel) {
+	v.RegisterStructValidationCtx(func(ctx context.Context, sl validator.StructLevel) {
 		driveItemInvite := sl.Current().Interface().(libregraph.DriveItemInvite)
-
+		rolesAndActions(ctx, sl, driveItemInvite.Roles, driveItemInvite.LibreGraphPermissionsActions, false)
-		rolesAndActions(sl, driveItemInvite.Roles, driveItemInvite.LibreGraphPermissionsActions, false)
 	}, s)
 }
 
@@ -52,7 +56,7 @@ func libregraphPermission(v *validator.Validate) {
 	v.RegisterStructValidationMapRules(map[string]string{
 		"Roles": "max=1",
 	}, s)
-	v.RegisterStructValidation(func(sl validator.StructLevel) {
+	v.RegisterStructValidationCtx(func(ctx context.Context, sl validator.StructLevel) {
 		permission := sl.Current().Interface().(libregraph.Permission)
 
 		if _, ok := permission.GetIdOk(); ok {
@@ -63,14 +67,13 @@ func libregraphPermission(v *validator.Validate) {
 			sl.ReportError(permission.HasPassword, "hasPassword", "HasPassword", "readonly", "")
 		}
 
-		rolesAndActions(sl, permission.Roles, permission.LibreGraphPermissionsActions, true)
+		rolesAndActions(ctx, sl, permission.Roles, permission.LibreGraphPermissionsActions, true)
 	}, s)
 }
 
-func rolesAndActions(sl validator.StructLevel, roles, actions []string, allowEmpty bool) {
+func rolesAndActions(ctx context.Context, sl validator.StructLevel, roles, actions []string, allowEmpty bool) {
 	totalRoles := len(roles)
 	totalActions := len(actions)
-
 	switch {
 	case allowEmpty && totalRoles == 0 && totalActions == 0:
 		break
@@ -83,12 +86,16 @@ func rolesAndActions(sl validator.StructLevel, roles, actions []string, allowEmp
 
 	var availableRoles []string
 	var availableActions []string
-	for _, definition := range append(
+	var definitions []*libregraph.UnifiedRoleDefinition
-		// fixMe: why twice!?
+
-		// fixMe: should we consider all roles or only the ones that are enabled?
+	switch roles, ok := ctx.Value(_contextRoleIDsValueKey).([]string); {
-		unifiedrole.GetBuiltinRoleDefinitionList(),
+	case ok:
-		unifiedrole.GetBuiltinRoleDefinitionList()...,
+		definitions = unifiedrole.GetBuiltinRoleDefinitionList(unifiedrole.RoleFilterIDs(roles...))
-	) {
+	default:
+		definitions = unifiedrole.GetBuiltinRoleDefinitionList()
+	}
+
+	for _, definition := range definitions {
 		if slices.Contains(availableRoles, definition.GetId()) {
 			continue
 		}
@@ -122,3 +129,8 @@ func rolesAndActions(sl validator.StructLevel, roles, actions []string, allowEmp
 		sl.ReportError(actions, "LibreGraphPermissionsActions", "LibreGraphPermissionsActions", "available_action", "")
 	}
 }
+
+// ContextWithAllowedRoleIDs returns a new context which includes the allowed role IDs.
+func ContextWithAllowedRoleIDs(ctx context.Context, rolesIds []string) context.Context {
+	return context.WithValue(ctx, _contextRoleIDsValueKey, rolesIds)
+}