RBAC: Allow role registration for plugins (#57387)

* Picking role registration from OnCall POC branch

* Fix test

* Remove include actions from this PR

* Removing unused permission

* Adding test to DeclarePluginRoles

* Add testcase to RegisterFixed role

* Additional test case

* Adding tests to validate plugins roles

* Add test to plugin loader

* Nit.

* Scuemata validation

* Changing the design to decouple accesscontrol from plugin management

Co-authored-by: Kalle Persson <kalle.persson@grafana.com>

* Fixing tests

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Add missing files

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Remove feature toggle check from loader

* Remove feature toggleimport

* Feedback

Co-Authored-By: marefr <marcus.efraimsson@gmail.com>

* Fix test'

* Make plugins.RoleRegistry interface typed

* Remove comment question

* No need for json tags anymore

* Nit. log

* Adding the schema validation

* Remove group to take plugin Name instead

* Revert sqlstore -> db

* Nit.

* Nit. on tests

Co-authored-by: ievaVasiljeva <ieva.vasiljeva@grafana.com>

* Update pkg/services/accesscontrol/plugins.go

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

* Log message

Co-Authored-By: marefr <marcus.efraimsson@gmail.com>

* Log message

Co-Authored-By: marefr <marcus.efraimsson@gmail.com>

* Remove unecessary method. Update test name.

Co-authored-by: ievaVasiljeva <ieva.vasiljeva@grafana.com>

* Fix linting

* Update cue descriptions

* Fix test

Co-authored-by: Kalle Persson <kalle.persson@grafana.com>
Co-authored-by: Jguer <joao.guerreiro@grafana.com>
Co-authored-by: marefr <marcus.efraimsson@gmail.com>
Co-authored-by: ievaVasiljeva <ieva.vasiljeva@grafana.com>

pkg/services/accesscontrol/acimpl/service.go

@@ -12,20 +12,26 @@ import (
 	"github.com/grafana/grafana/pkg/infra/localcache"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/metrics"
+	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/api"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/database"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/pluginutils"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
+var _ plugins.RoleRegistry = &Service{}
+
 const (
 	cacheTTL = 10 * time.Second
 )
 
-func ProvideService(cfg *setting.Cfg, store db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService) (*Service, error) {
-	service := ProvideOSSService(cfg, database.ProvideService(store), cache)
+func ProvideService(cfg *setting.Cfg, store db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
+	features *featuremgmt.FeatureManager) (*Service, error) {
+	service := ProvideOSSService(cfg, database.ProvideService(store), cache, features)
 
 	if !accesscontrol.IsDisabled(cfg) {
 		api.NewAccessControlAPI(routeRegister, service).RegisterAPIEndpoints()
@@ -37,13 +43,14 @@ func ProvideService(cfg *setting.Cfg, store db.DB, routeRegister routing.RouteRe
 	return service, nil
 }
 
-func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService) *Service {
+func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService, features *featuremgmt.FeatureManager) *Service {
 	s := &Service{
-		cfg:   cfg,
-		store: store,
-		log:   log.New("accesscontrol.service"),
-		cache: cache,
-		roles: accesscontrol.BuildBasicRoleDefinitions(),
+		cfg:      cfg,
+		store:    store,
+		log:      log.New("accesscontrol.service"),
+		cache:    cache,
+		roles:    accesscontrol.BuildBasicRoleDefinitions(),
+		features: features,
 	}
 
 	return s
@@ -62,6 +69,7 @@ type Service struct {
 	cache         *localcache.CacheService
 	registrations accesscontrol.RegistrationList
 	roles         map[string]*accesscontrol.RoleDTO
+	features      *featuremgmt.FeatureManager
 }
 
 func (s *Service) GetUsageStats(_ context.Context) map[string]interface{} {
@@ -198,3 +206,33 @@ func permissionCacheKey(user *user.SignedInUser) (string, error) {
 	}
 	return fmt.Sprintf("rbac-permissions-%s", key), nil
 }
+
+// DeclarePluginRoles allow the caller to declare, to the service, plugin roles and their assignments
+// to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
+func (s *Service) DeclarePluginRoles(_ context.Context, ID, name string, regs []plugins.RoleRegistration) error {
+	// If accesscontrol is disabled no need to register roles
+	if accesscontrol.IsDisabled(s.cfg) {
+		return nil
+	}
+
+	// Protect behind feature toggle
+	if !s.features.IsEnabled(featuremgmt.FlagAccessControlOnCall) {
+		return nil
+	}
+
+	acRegs := pluginutils.ToRegistrations(name, regs)
+	for _, r := range acRegs {
+		if err := pluginutils.ValidatePluginRole(ID, r.Role); err != nil {
+			return err
+		}
+
+		if err := accesscontrol.ValidateBuiltInRoles(r.Grants); err != nil {
+			return err
+		}
+
+		s.log.Debug("Registering plugin role", "role", r.Role.Name)
+		s.registrations.Append(r)
+	}
+
+	return nil
+}

pkg/services/accesscontrol/acimpl/service_test.go

@@ -12,8 +12,10 @@ import (
 	"github.com/grafana/grafana/pkg/infra/localcache"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/database"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 )
@@ -29,6 +31,7 @@ func setupTestEnv(t testing.TB) *Service {
 		registrations: accesscontrol.RegistrationList{},
 		store:         database.ProvideService(db.InitTestDB(t)),
 		roles:         accesscontrol.BuildBasicRoleDefinitions(),
+		features:      featuremgmt.WithFeatures(),
 	}
 	require.NoError(t, ac.RegisterFixedRoles(context.Background()))
 	return ac
@@ -62,6 +65,7 @@ func TestUsageMetrics(t *testing.T) {
 				db.InitTestDB(t),
 				routing.NewRouteRegister(),
 				localcache.ProvideService(),
+				featuremgmt.WithFeatures(),
 			)
 			require.NoError(t, errInitAc)
 			assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"])
@@ -84,9 +88,7 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 			name: "should add registration",
 			registrations: []accesscontrol.RoleRegistration{
 				{
-					Role: accesscontrol.RoleDTO{
-						Name: "fixed:test:test",
-					},
+					Role:   accesscontrol.RoleDTO{Name: "fixed:test:test"},
 					Grants: []string{"Admin"},
 				},
 			},
@@ -96,9 +98,7 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 			name: "should fail registration invalid role name",
 			registrations: []accesscontrol.RoleRegistration{
 				{
-					Role: accesscontrol.RoleDTO{
-						Name: "custom:test:test",
-					},
+					Role:   accesscontrol.RoleDTO{Name: "custom:test:test"},
 					Grants: []string{"Admin"},
 				},
 			},
@@ -106,12 +106,10 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 			err:     accesscontrol.ErrFixedRolePrefixMissing,
 		},
 		{
-			name: "should fail registration invalid builtin role assignment",
+			name: "should fail registration invalid basic role assignment",
 			registrations: []accesscontrol.RoleRegistration{
 				{
-					Role: accesscontrol.RoleDTO{
-						Name: "fixed:test:test",
-					},
+					Role:   accesscontrol.RoleDTO{Name: "fixed:test:test"},
 					Grants: []string{"WrongAdmin"},
 				},
 			},
@@ -122,15 +120,11 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 			name: "should add multiple registrations at once",
 			registrations: []accesscontrol.RoleRegistration{
 				{
-					Role: accesscontrol.RoleDTO{
-						Name: "fixed:test:test",
-					},
+					Role:   accesscontrol.RoleDTO{Name: "fixed:test:test"},
 					Grants: []string{"Admin"},
 				},
 				{
-					Role: accesscontrol.RoleDTO{
-						Name: "fixed:test2:test2",
-					},
+					Role:   accesscontrol.RoleDTO{Name: "fixed:test2:test2"},
 					Grants: []string{"Admin"},
 				},
 			},
@@ -164,6 +158,132 @@ func TestService_DeclareFixedRoles(t *testing.T) {
 	}
 }
 
+func TestService_DeclarePluginRoles(t *testing.T) {
+	tests := []struct {
+		name          string
+		pluginID      string
+		registrations []plugins.RoleRegistration
+		wantErr       bool
+		err           error
+	}{
+		{
+			name:    "should work with empty list",
+			wantErr: false,
+		},
+		{
+			name:     "should add registration",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role:   plugins.Role{Name: "plugins:test-app:test"},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name:     "should fail registration invalid role name",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role:   plugins.Role{Name: "invalid.plugins:test-app:test"},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: true,
+			err:     &accesscontrol.ErrorInvalidRole{},
+		},
+		{
+			name:     "should add registration with valid permissions",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role: plugins.Role{
+						Name: "plugins:test-app:test",
+						Permissions: []plugins.Permission{
+							{Action: "plugins.app:access"},
+							{Action: "test-app:read"},
+							{Action: "test-app.resource:read"},
+						},
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name:     "should fail registration invalid permission action",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role: plugins.Role{
+						Name: "plugins:test-app:test",
+						Permissions: []plugins.Permission{
+							{Action: "invalid.test-app.resource:read"},
+						},
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: true,
+			err:     &accesscontrol.ErrorInvalidRole{},
+		},
+		{
+			name:     "should fail registration invalid basic role assignment",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role:   plugins.Role{Name: "plugins:test-app:test"},
+					Grants: []string{"WrongAdmin"},
+				},
+			},
+			wantErr: true,
+			err:     accesscontrol.ErrInvalidBuiltinRole,
+		},
+		{
+			name:     "should add multiple registrations at once",
+			pluginID: "test-app",
+			registrations: []plugins.RoleRegistration{
+				{
+					Role:   plugins.Role{Name: "plugins:test-app:test"},
+					Grants: []string{"Admin"},
+				},
+				{
+					Role:   plugins.Role{Name: "plugins:test-app:test2"},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ac := setupTestEnv(t)
+			ac.features = featuremgmt.WithFeatures(featuremgmt.FlagAccessControlOnCall)
+
+			// Reset the registations
+			ac.registrations = accesscontrol.RegistrationList{}
+
+			// Test
+			err := ac.DeclarePluginRoles(context.Background(), tt.pluginID, tt.pluginID, tt.registrations)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.ErrorIs(t, err, tt.err)
+				return
+			}
+			require.NoError(t, err)
+
+			registrationCnt := 0
+			ac.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
+				registrationCnt++
+				return true
+			})
+			assert.Equal(t, len(tt.registrations), registrationCnt,
+				"expected service registration list to contain all test registrations")
+		})
+	}
+}
+
 func TestService_RegisterFixedRoles(t *testing.T) {
 	tests := []struct {
 		name          string
@@ -210,6 +330,29 @@ func TestService_RegisterFixedRoles(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "should register and assign fixed and plugins roles",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Name:        "plugins:test-app:test",
+						Permissions: []accesscontrol.Permission{{Action: "test-app:test"}},
+					},
+					Grants: []string{"Editor"},
+				},
+				{
+					Role: accesscontrol.RoleDTO{
+						Name: "fixed:test2:test2",
+						Permissions: []accesscontrol.Permission{
+							{Action: "test:test2"},
+							{Action: "test:test3", Scope: "test:*"},
+						},
+					},
+					Grants: []string{"Viewer"},
+				},
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {

pkg/services/accesscontrol/errors.go

@@ -1,10 +1,46 @@
 package accesscontrol
 
-import "errors"
+import (
+	"errors"
+	"fmt"
+)
 
 var (
 	ErrFixedRolePrefixMissing = errors.New("fixed role should be prefixed with '" + FixedRolePrefix + "'")
 	ErrInvalidBuiltinRole     = errors.New("built-in role is not valid")
 	ErrInvalidScope           = errors.New("invalid scope")
 	ErrResolverNotFound       = errors.New("no resolver found")
+	ErrPluginIDRequired       = errors.New("plugin ID is required")
 )
+
+type ErrorInvalidRole struct{}
+
+func (e *ErrorInvalidRole) Error() string {
+	return "role is invalid"
+}
+
+type ErrorRolePrefixMissing struct {
+	Role     string
+	Prefixes []string
+}
+
+func (e *ErrorRolePrefixMissing) Error() string {
+	return fmt.Sprintf("expected role '%s' to be prefixed with any of '%v'", e.Role, e.Prefixes)
+}
+
+func (e *ErrorRolePrefixMissing) Unwrap() error {
+	return &ErrorInvalidRole{}
+}
+
+type ErrorActionPrefixMissing struct {
+	Action   string
+	Prefixes []string
+}
+
+func (e *ErrorActionPrefixMissing) Error() string {
+	return fmt.Sprintf("expected action '%s' to be prefixed with any of '%v'", e.Action, e.Prefixes)
+}
+
+func (e *ErrorActionPrefixMissing) Unwrap() error {
+	return &ErrorInvalidRole{}
+}

pkg/services/accesscontrol/mock/mock.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/user"
 )
@@ -12,6 +13,7 @@ import (
 type fullAccessControl interface {
 	accesscontrol.AccessControl
 	accesscontrol.Service
+	plugins.RoleRegistry
 	RegisterFixedRoles(context.Context) error
 }
 
@@ -20,6 +22,7 @@ type Calls struct {
 	GetUserPermissions             []interface{}
 	IsDisabled                     []interface{}
 	DeclareFixedRoles              []interface{}
+	DeclarePluginRoles             []interface{}
 	GetUserBuiltInRoles            []interface{}
 	RegisterFixedRoles             []interface{}
 	RegisterAttributeScopeResolver []interface{}
@@ -42,6 +45,7 @@ type Mock struct {
 	GetUserPermissionsFunc             func(context.Context, *user.SignedInUser, accesscontrol.Options) ([]accesscontrol.Permission, error)
 	IsDisabledFunc                     func() bool
 	DeclareFixedRolesFunc              func(...accesscontrol.RoleRegistration) error
+	DeclarePluginRolesFunc             func(context.Context, string, string, []plugins.RoleRegistration) error
 	GetUserBuiltInRolesFunc            func(user *user.SignedInUser) []string
 	RegisterFixedRolesFunc             func() error
 	RegisterScopeAttributeResolverFunc func(string, accesscontrol.ScopeAttributeResolver)
@@ -169,6 +173,18 @@ func (m *Mock) RegisterFixedRoles(ctx context.Context) error {
 	return nil
 }
 
+// DeclarePluginRoles allow the caller to declare, to the service, plugin roles and their
+// assignments to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
+// This mock returns no error unless an override is provided.
+func (m *Mock) DeclarePluginRoles(ctx context.Context, ID, name string, regs []plugins.RoleRegistration) error {
+	m.Calls.DeclarePluginRoles = append(m.Calls.DeclarePluginRoles, []interface{}{ctx, ID, name, regs})
+	// Use override if provided
+	if m.DeclarePluginRolesFunc != nil {
+		return m.DeclarePluginRolesFunc(ctx, ID, name, regs)
+	}
+	return nil
+}
+
 func (m *Mock) RegisterScopeAttributeResolver(scopePrefix string, resolver accesscontrol.ScopeAttributeResolver) {
 	m.scopeResolvers.AddScopeAttributeResolver(scopePrefix, resolver)
 	m.Calls.RegisterAttributeScopeResolver = append(m.Calls.RegisterAttributeScopeResolver, []struct{}{})

pkg/services/accesscontrol/models.go

@@ -126,6 +126,10 @@ func (r *RoleDTO) IsFixed() bool {
 	return strings.HasPrefix(r.Name, FixedRolePrefix)
 }
 
+func (r *RoleDTO) IsPlugin() bool {
+	return strings.HasPrefix(r.Name, PluginRolePrefix)
+}
+
 func (r *RoleDTO) IsBasic() bool {
 	return strings.HasPrefix(r.Name, BasicRolePrefix) || strings.HasPrefix(r.UID, BasicRoleUIDPrefix)
 }
@@ -273,6 +277,7 @@ const (
 	FixedRolePrefix    = "fixed:"
 	ManagedRolePrefix  = "managed:"
 	BasicRolePrefix    = "basic:"
+	PluginRolePrefix   = "plugins:"
 	BasicRoleUIDPrefix = "basic_"
 	RoleGrafanaAdmin   = "Grafana Admin"
 

pkg/services/accesscontrol/pluginutils/utils.go

@@ -0,0 +1,62 @@
+package pluginutils
+
+import (
+	"strings"
+
+	"github.com/grafana/grafana/pkg/plugins"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
+)
+
+// ValidatePluginPermissions errors when a permission does not match expected pattern for plugins
+func ValidatePluginPermissions(pluginID string, permissions []ac.Permission) error {
+	for i := range permissions {
+		if permissions[i].Action != plugins.ActionAppAccess &&
+			!strings.HasPrefix(permissions[i].Action, pluginID+":") &&
+			!strings.HasPrefix(permissions[i].Action, pluginID+".") {
+			return &ac.ErrorActionPrefixMissing{Action: permissions[i].Action,
+				Prefixes: []string{plugins.ActionAppAccess, pluginID + ":", pluginID + "."}}
+		}
+	}
+
+	return nil
+}
+
+// ValidatePluginRole errors when a plugin role does not match expected pattern
+// or doesn't have permissions matching the expected pattern.
+func ValidatePluginRole(pluginID string, role ac.RoleDTO) error {
+	if pluginID == "" {
+		return ac.ErrPluginIDRequired
+	}
+	if !strings.HasPrefix(role.Name, ac.PluginRolePrefix+pluginID+":") {
+		return &ac.ErrorRolePrefixMissing{Role: role.Name, Prefixes: []string{ac.PluginRolePrefix + pluginID + ":"}}
+	}
+
+	return ValidatePluginPermissions(pluginID, role.Permissions)
+}
+
+func ToRegistrations(pluginName string, regs []plugins.RoleRegistration) []ac.RoleRegistration {
+	res := make([]ac.RoleRegistration, 0, len(regs))
+	for i := range regs {
+		res = append(res, ac.RoleRegistration{
+			Role: ac.RoleDTO{
+				Version:     1,
+				Name:        regs[i].Role.Name,
+				DisplayName: regs[i].Role.DisplayName,
+				Description: regs[i].Role.Description,
+				Group:       pluginName,
+				Permissions: toPermissions(regs[i].Role.Permissions),
+				OrgID:       ac.GlobalOrgID,
+			},
+			Grants: regs[i].Grants,
+		})
+	}
+	return res
+}
+
+func toPermissions(perms []plugins.Permission) []ac.Permission {
+	res := make([]ac.Permission, 0, len(perms))
+	for i := range perms {
+		res = append(res, ac.Permission{Action: perms[i].Action, Scope: perms[i].Scope})
+	}
+	return res
+}

pkg/services/accesscontrol/pluginutils/utils_test.go

@@ -0,0 +1,142 @@
+package pluginutils
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/plugins"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/stretchr/testify/require"
+)
+
+func TestToRegistrations(t *testing.T) {
+	tests := []struct {
+		name string
+		regs []plugins.RoleRegistration
+		want []ac.RoleRegistration
+	}{
+		{
+			name: "no registration",
+			regs: nil,
+			want: []ac.RoleRegistration{},
+		},
+		{
+			name: "registration gets converted successfully",
+			regs: []plugins.RoleRegistration{
+				{
+					Role: plugins.Role{
+						Name:        "test:name",
+						DisplayName: "Test",
+						Description: "Test",
+						Permissions: []plugins.Permission{
+							{Action: "test:action"},
+							{Action: "test:action", Scope: "test:scope"},
+						},
+					},
+					Grants: []string{"Admin", "Editor"},
+				},
+				{
+					Role: plugins.Role{
+						Name:        "test:name",
+						Permissions: []plugins.Permission{},
+					},
+				},
+			},
+			want: []ac.RoleRegistration{
+				{
+					Role: ac.RoleDTO{
+						Version:     1,
+						Name:        "test:name",
+						DisplayName: "Test",
+						Description: "Test",
+						Group:       "PluginName",
+						Permissions: []ac.Permission{
+							{Action: "test:action"},
+							{Action: "test:action", Scope: "test:scope"},
+						},
+						OrgID: ac.GlobalOrgID,
+					},
+					Grants: []string{"Admin", "Editor"},
+				},
+				{
+					Role: ac.RoleDTO{
+						Version:     1,
+						Name:        "test:name",
+						Group:       "PluginName",
+						Permissions: []ac.Permission{},
+						OrgID:       ac.GlobalOrgID,
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ToRegistrations("PluginName", tt.regs)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestValidatePluginRole(t *testing.T) {
+	tests := []struct {
+		name     string
+		pluginID string
+		role     ac.RoleDTO
+		wantErr  error
+	}{
+		{
+			name:     "empty",
+			pluginID: "",
+			role:     ac.RoleDTO{Name: "plugins::"},
+			wantErr:  ac.ErrPluginIDRequired,
+		},
+		{
+			name:     "invalid name",
+			pluginID: "test-app",
+			role:     ac.RoleDTO{Name: "test-app:reader"},
+			wantErr:  &ac.ErrorInvalidRole{},
+		},
+		{
+			name:     "invalid id in name",
+			pluginID: "test-app",
+			role:     ac.RoleDTO{Name: "plugins:test-app2:reader"},
+			wantErr:  &ac.ErrorInvalidRole{},
+		},
+		{
+			name:     "valid name",
+			pluginID: "test-app",
+			role:     ac.RoleDTO{Name: "plugins:test-app:reader"},
+		},
+		{
+			name:     "invalid permission",
+			pluginID: "test-app",
+			role: ac.RoleDTO{
+				Name:        "plugins:test-app:reader",
+				Permissions: []ac.Permission{{Action: "invalidtest-app:read"}},
+			},
+			wantErr: &ac.ErrorInvalidRole{},
+		},
+		{
+			name:     "valid permissions",
+			pluginID: "test-app",
+			role: ac.RoleDTO{
+				Name: "plugins:test-app:reader",
+				Permissions: []ac.Permission{
+					{Action: "plugins.app:access"},
+					{Action: "test-app:read"},
+					{Action: "test-app.resources:read"},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidatePluginRole(tt.pluginID, tt.role)
+			if tt.wantErr != nil {
+				require.ErrorIs(t, err, tt.wantErr)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}

pkg/services/accesscontrol/resolvers.go

@@ -44,7 +44,7 @@ type Resolvers struct {
 }
 
 func (s *Resolvers) AddScopeAttributeResolver(prefix string, resolver ScopeAttributeResolver) {
-	s.log.Debug("adding scope attribute resolver for '%v'", prefix)
+	s.log.Debug("adding scope attribute resolver", "prefix", prefix)
 	s.attributeResolvers[prefix] = resolver
 }