diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 77e44244bf0..8d45bfb2677 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -406,7 +406,9 @@ func (s *Service) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistrat } for i := range r.Role.Permissions { - s.permRegistry.RegisterPermission(r.Role.Permissions[i].Action, r.Role.Permissions[i].Scope) + if err := s.permRegistry.RegisterPermission(r.Role.Permissions[i].Action, r.Role.Permissions[i].Scope); err != nil { + return err + } } s.registrations.Append(r) @@ -464,7 +466,9 @@ func (s *Service) DeclarePluginRoles(ctx context.Context, ID, name string, regs for i := range r.Role.Permissions { // Register plugin actions and their possible scopes for permission validation s.permRegistry.RegisterPluginScope(r.Role.Permissions[i].Scope) - s.permRegistry.RegisterPermission(r.Role.Permissions[i].Action, r.Role.Permissions[i].Scope) + if err := s.permRegistry.RegisterPermission(r.Role.Permissions[i].Action, r.Role.Permissions[i].Scope); err != nil { + return err + } } s.log.Debug("Registering plugin role", "role", r.Role.Name) diff --git a/pkg/services/accesscontrol/permreg/permreg.go b/pkg/services/accesscontrol/permreg/permreg.go index ca1b8ba1fd1..cb3cc4ba5b5 100644 --- a/pkg/services/accesscontrol/permreg/permreg.go +++ b/pkg/services/accesscontrol/permreg/permreg.go @@ -14,6 +14,8 @@ var ( ErrUnknownActionTplt = "unknown action: {{.Public.Action}}, was not found in the list of valid actions" ErrBaseUnknownAction = errutil.BadRequest("permreg.unknown-action").MustTemplate(ErrUnknownActionTplt, errutil.WithPublic(ErrUnknownActionTplt)) + + ErrBaseUnknownKind = errutil.BadRequest("permreg.unknown-kind").MustTemplate("unknown kind: {{.Public.Kind}}") ) func ErrInvalidScope(scope string, action string, validScopePrefixes PrefixSet) error { @@ -28,6 +30,10 @@ func ErrUnknownAction(action string) error { return ErrBaseUnknownAction.Build(errutil.TemplateData{Public: map[string]any{"Action": action}}) } +func ErrUnknownKind(kind string) error { + return ErrBaseUnknownKind.Build(errutil.TemplateData{Public: map[string]any{"Kind": kind}}) +} + func generateValidScopeFormats(acceptedScopePrefixes PrefixSet) []string { if len(acceptedScopePrefixes) == 0 { return []string{} @@ -48,7 +54,7 @@ func generateValidScopeFormats(acceptedScopePrefixes PrefixSet) []string { type PermissionRegistry interface { RegisterPluginScope(scope string) - RegisterPermission(action, scope string) + RegisterPermission(action, scope string) error IsPermissionValid(action, scope string) error GetScopePrefixes(action string) (PrefixSet, bool) } @@ -87,6 +93,7 @@ func newPermissionRegistry() *permissionRegistry { "global.users": "global.users:id:", "roles": "roles:uid:", "services": "services:", + "receivers": "receivers:uid:", } return &permissionRegistry{ actionScopePrefixes: make(map[string]PrefixSet, 200), @@ -101,42 +108,47 @@ func (pr *permissionRegistry) RegisterPluginScope(scope string) { } scopeParts := strings.Split(scope, ":") + kind := scopeParts[0] + // If the scope is already registered, return - if _, found := pr.kindScopePrefix[scopeParts[0]]; found { + if _, found := pr.kindScopePrefix[kind]; found { return } // If the scope contains an attribute part, register the kind and attribute if len(scopeParts) > 2 { - kind, attr := scopeParts[0], scopeParts[1] + attr := scopeParts[1] pr.kindScopePrefix[kind] = kind + ":" + attr + ":" pr.logger.Debug("registered scope prefix", "kind", kind, "scope_prefix", kind+":"+attr+":") return } - pr.logger.Debug("registered scope prefix", "kind", scopeParts[0], "scope_prefix", scopeParts[0]+":") - pr.kindScopePrefix[scopeParts[0]] = scopeParts[0] + ":" + pr.logger.Debug("registered scope prefix", "kind", kind, "scope_prefix", kind+":") + pr.kindScopePrefix[kind] = kind + ":" } -func (pr *permissionRegistry) RegisterPermission(action, scope string) { - if _, ok := pr.actionScopePrefixes[action]; !ok { - pr.actionScopePrefixes[action] = PrefixSet{} +func (pr *permissionRegistry) RegisterPermission(action, scope string) error { + if _, ok := pr.actionScopePrefixes[action]; ok { + // action already registered + return nil } + pr.actionScopePrefixes[action] = PrefixSet{} if scope == "" { // scopeless action - return + return nil } kind := strings.Split(scope, ":")[0] scopePrefix, ok := pr.kindScopePrefix[kind] if !ok { - pr.logger.Warn("unknown scope prefix", "scope", scope) - return + pr.logger.Error("unknown kind: please update `kindScopePrefix` with the correct scope prefix", "kind", kind) + return ErrUnknownKind(kind) } // Add a new entry in case the scope is not empty pr.actionScopePrefixes[action][scopePrefix] = true + return nil } func (pr *permissionRegistry) IsPermissionValid(action, scope string) error { diff --git a/pkg/services/accesscontrol/permreg/permreg_test.go b/pkg/services/accesscontrol/permreg/permreg_test.go index 4d78e0ef9a6..0f16555c541 100644 --- a/pkg/services/accesscontrol/permreg/permreg_test.go +++ b/pkg/services/accesscontrol/permreg/permreg_test.go @@ -51,7 +51,7 @@ func Test_permissionRegistry_RegisterPermission(t *testing.T) { scope string wantKind string wantPrefixSet PrefixSet - wantSkip bool + wantErr bool }{ { name: "register folders read", @@ -67,16 +67,31 @@ func Test_permissionRegistry_RegisterPermission(t *testing.T) { wantPrefixSet: PrefixSet{}, }, { - name: "register an action on an unknown kind", - action: "unknown:action", - scope: "unknown:uid:*", - wantPrefixSet: PrefixSet{}, + name: "register an action on an unknown kind", + action: "unknown:action", + scope: "unknown:uid:*", + wantErr: true, + }, + { + name: "register an action that is already registered", + action: "already:registered", + scope: "already:uid:*", + wantPrefixSet: PrefixSet{"already:uid:": true}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pr := newPermissionRegistry() - pr.RegisterPermission(tt.action, tt.scope) + + // Pretend that an action is registered + pr.actionScopePrefixes["already:registered"] = PrefixSet{"already:uid:": true} + + err := pr.RegisterPermission(tt.action, tt.scope) + if tt.wantErr { + require.Error(t, err) + return + } + got, ok := pr.actionScopePrefixes[tt.action] require.True(t, ok) for k, v := range got { @@ -88,8 +103,10 @@ func Test_permissionRegistry_RegisterPermission(t *testing.T) { func Test_permissionRegistry_IsPermissionValid(t *testing.T) { pr := newPermissionRegistry() - pr.RegisterPermission("folders:read", "folders:uid:") - pr.RegisterPermission("test-app.settings:read", "") + err := pr.RegisterPermission("folders:read", "folders:uid:") + require.NoError(t, err) + err = pr.RegisterPermission("test-app.settings:read", "") + require.NoError(t, err) tests := []struct { name string @@ -166,8 +183,10 @@ func Test_permissionRegistry_IsPermissionValid(t *testing.T) { func Test_permissionRegistry_GetScopePrefixes(t *testing.T) { pr := newPermissionRegistry() - pr.RegisterPermission("folders:read", "folders:uid:") - pr.RegisterPermission("test-app.settings:read", "") + err := pr.RegisterPermission("folders:read", "folders:uid:") + require.NoError(t, err) + err = pr.RegisterPermission("test-app.settings:read", "") + require.NoError(t, err) tests := []struct { name string diff --git a/pkg/services/accesscontrol/permreg/test/testreg.go b/pkg/services/accesscontrol/permreg/test/testreg.go index 7b5210dd2e6..43f9294dd87 100644 --- a/pkg/services/accesscontrol/permreg/test/testreg.go +++ b/pkg/services/accesscontrol/permreg/test/testreg.go @@ -1,22 +1,38 @@ package test -import "github.com/grafana/grafana/pkg/services/accesscontrol/permreg" +import ( + "testing" -func ProvidePermissionRegistry() permreg.PermissionRegistry { + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/accesscontrol/permreg" +) + +func ProvidePermissionRegistry(t *testing.T) permreg.PermissionRegistry { permReg := permreg.ProvidePermissionRegistry() // Test core permissions - permReg.RegisterPermission("datasources:read", "datasources:uid:") - permReg.RegisterPermission("dashboards:read", "dashboards:uid:") - permReg.RegisterPermission("dashboards:read", "folders:uid:") - permReg.RegisterPermission("folders:read", "folders:uid:") + err := permReg.RegisterPermission("datasources:read", "datasources:uid:") + require.NoError(t, err) + err = permReg.RegisterPermission("dashboards:read", "dashboards:uid:") + require.NoError(t, err) + err = permReg.RegisterPermission("dashboards:read", "folders:uid:") + require.NoError(t, err) + err = permReg.RegisterPermission("folders:read", "folders:uid:") + require.NoError(t, err) // Test plugins permissions - permReg.RegisterPermission("plugins.app:access", "plugins:id:") + err = permReg.RegisterPermission("plugins.app:access", "plugins:id:") + require.NoError(t, err) // App - permReg.RegisterPermission("test-app:read", "") - permReg.RegisterPermission("test-app.settings:read", "") - permReg.RegisterPermission("test-app.projects:read", "") + err = permReg.RegisterPermission("test-app:read", "") + require.NoError(t, err) + err = permReg.RegisterPermission("test-app.settings:read", "") + require.NoError(t, err) + err = permReg.RegisterPermission("test-app.projects:read", "") + require.NoError(t, err) // App 1 - permReg.RegisterPermission("test-app1.catalog:read", "") - permReg.RegisterPermission("test-app1.announcements:read", "") + err = permReg.RegisterPermission("test-app1.catalog:read", "") + require.NoError(t, err) + err = permReg.RegisterPermission("test-app1.announcements:read", "") + require.NoError(t, err) return permReg }