Revert "Revert "AccessControl: Implement a way to register fixed roles (#35641)" (#37397)" (#37535)

This reverts commit 55efeb0c02.

pkg/api/admin_provisioning_test.go

@@ -0,0 +1,169 @@
+package api
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/provisioning"
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/stretchr/testify/assert"
+)
+
+type reloadProvisioningTestCase struct {
+	desc         string
+	url          string
+	expectedCode int
+	expectedBody string
+	permissions  []*accesscontrol.Permission
+	exit         bool
+	checkCall    func(mock provisioning.ProvisioningServiceMock)
+}
+
+func TestAPI_AdminProvisioningReload_AccessControl(t *testing.T) {
+	tests := []reloadProvisioningTestCase{
+		{
+			desc:         "should work for dashboards with specific scope",
+			expectedCode: http.StatusOK,
+			expectedBody: `{"message":"Dashboards config reloaded"}`,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersDashboards,
+				},
+			},
+			url: "/api/admin/provisioning/dashboards/reload",
+			checkCall: func(mock provisioning.ProvisioningServiceMock) {
+				assert.Len(t, mock.Calls.ProvisionDashboards, 1)
+			},
+		},
+		{
+			desc:         "should work for dashboards with broader scope",
+			expectedCode: http.StatusOK,
+			expectedBody: `{"message":"Dashboards config reloaded"}`,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersAll,
+				},
+			},
+			url: "/api/admin/provisioning/dashboards/reload",
+			checkCall: func(mock provisioning.ProvisioningServiceMock) {
+				assert.Len(t, mock.Calls.ProvisionDashboards, 1)
+			},
+		},
+		{
+			desc:         "should fail for dashboard with wrong scope",
+			expectedCode: http.StatusForbidden,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  "services:noservice",
+				},
+			},
+			url:  "/api/admin/provisioning/dashboards/reload",
+			exit: true,
+		},
+		{
+			desc:         "should fail for dashboard with no permission",
+			expectedCode: http.StatusForbidden,
+			url:          "/api/admin/provisioning/dashboards/reload",
+			exit:         true,
+		},
+		{
+			desc:         "should work for notifications with specific scope",
+			expectedCode: http.StatusOK,
+			expectedBody: `{"message":"Notifications config reloaded"}`,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersNotifications,
+				},
+			},
+			url: "/api/admin/provisioning/notifications/reload",
+			checkCall: func(mock provisioning.ProvisioningServiceMock) {
+				assert.Len(t, mock.Calls.ProvisionNotifications, 1)
+			},
+		},
+		{
+			desc:         "should fail for notifications with no permission",
+			expectedCode: http.StatusForbidden,
+			url:          "/api/admin/provisioning/notifications/reload",
+			exit:         true,
+		},
+		{
+			desc:         "should work for datasources with specific scope",
+			expectedCode: http.StatusOK,
+			expectedBody: `{"message":"Datasources config reloaded"}`,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersDatasources,
+				},
+			},
+			url: "/api/admin/provisioning/datasources/reload",
+			checkCall: func(mock provisioning.ProvisioningServiceMock) {
+				assert.Len(t, mock.Calls.ProvisionDatasources, 1)
+			},
+		},
+		{
+			desc:         "should fail for datasources with no permission",
+			expectedCode: http.StatusForbidden,
+			url:          "/api/admin/provisioning/datasources/reload",
+			exit:         true,
+		},
+		{
+			desc:         "should work for plugins with specific scope",
+			expectedCode: http.StatusOK,
+			expectedBody: `{"message":"Plugins config reloaded"}`,
+			permissions: []*accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersPlugins,
+				},
+			},
+			url: "/api/admin/provisioning/plugins/reload",
+			checkCall: func(mock provisioning.ProvisioningServiceMock) {
+				assert.Len(t, mock.Calls.ProvisionPlugins, 1)
+			},
+		},
+		{
+			desc:         "should fail for plugins with no permission",
+			expectedCode: http.StatusForbidden,
+			url:          "/api/admin/provisioning/plugins/reload",
+			exit:         true,
+		},
+	}
+
+	cfg := setting.NewCfg()
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
+
+			// Setup the mock
+			provisioningMock := provisioning.NewProvisioningServiceMock()
+			hs.ProvisioningService = provisioningMock
+
+			sc.resp = httptest.NewRecorder()
+			var err error
+			sc.req, err = http.NewRequest(http.MethodPost, test.url, nil)
+			assert.NoError(t, err)
+
+			sc.exec()
+
+			// Check return code
+			assert.Equal(t, test.expectedCode, sc.resp.Code)
+			if test.exit {
+				return
+			}
+
+			// Check body
+			assert.Equal(t, test.expectedBody, sc.resp.Body.String())
+
+			// Check we actually called the provisioning service
+			test.checkCall(*provisioningMock)
+		})
+	}
+}

pkg/api/api.go

@@ -443,10 +443,11 @@ func (hs *HTTPServer) registerRoutes() {
 		adminRoute.Get("/stats", authorize(reqGrafanaAdmin, accesscontrol.ActionServerStatsRead), routing.Wrap(AdminGetStats))
 		adminRoute.Post("/pause-all-alerts", reqGrafanaAdmin, bind(dtos.PauseAllAlertsCommand{}), routing.Wrap(PauseAllAlerts))
 
-		adminRoute.Post("/provisioning/dashboards/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadDashboards))
+		adminRoute.Post("/provisioning/dashboards/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersDashboards), routing.Wrap(hs.AdminProvisioningReloadDashboards))
-		adminRoute.Post("/provisioning/plugins/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadPlugins))
+		adminRoute.Post("/provisioning/plugins/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersPlugins), routing.Wrap(hs.AdminProvisioningReloadPlugins))
-		adminRoute.Post("/provisioning/datasources/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadDatasources))
+		adminRoute.Post("/provisioning/datasources/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersDatasources), routing.Wrap(hs.AdminProvisioningReloadDatasources))
-		adminRoute.Post("/provisioning/notifications/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadNotifications))
+		adminRoute.Post("/provisioning/notifications/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersNotifications), routing.Wrap(hs.AdminProvisioningReloadNotifications))
+
 		adminRoute.Post("/ldap/reload", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPConfigReload), routing.Wrap(hs.ReloadLDAPCfg))
 		adminRoute.Post("/ldap/sync/:id", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPUsersSync), routing.Wrap(hs.PostSyncUserWithLDAP))
 		adminRoute.Get("/ldap/:username", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPUsersRead), routing.Wrap(hs.GetUserFromLDAP))

pkg/api/common_test.go

@@ -253,6 +253,10 @@ func (f *fakeAccessControl) IsDisabled() bool {
 	return f.isDisabled
 }
 
+func (f *fakeAccessControl) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistration) error {
+	return nil
+}
+
 func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url string, permissions []*accesscontrol.Permission) (*scenarioContext, *HTTPServer) {
 	cfg.FeatureToggles = make(map[string]bool)
 	cfg.FeatureToggles["accesscontrol"] = true

pkg/api/http_server.go

@@ -117,8 +117,7 @@ func (hs *HTTPServer) Init() error {
 
 	hs.macaron = hs.newMacaron()
 	hs.registerRoutes()
-
+	return hs.declareFixedRoles()
-	return nil
 }
 
 func (hs *HTTPServer) AddMiddleware(middleware macaron.Handler) {

pkg/api/roles.go

@@ -0,0 +1,41 @@
+package api
+
+import (
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+)
+
+// API related actions
+const (
+	ActionProvisioningReload = "provisioning:reload"
+)
+
+// API related scopes
+const (
+	ScopeProvisionersAll           = "provisioners:*"
+	ScopeProvisionersDashboards    = "provisioners:dashboards"
+	ScopeProvisionersPlugins       = "provisioners:plugins"
+	ScopeProvisionersDatasources   = "provisioners:datasources"
+	ScopeProvisionersNotifications = "provisioners:notifications"
+)
+
+// declareFixedRoles declares to the AccessControl service fixed roles and their
+// grants to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
+// that HTTPServer needs
+func (hs *HTTPServer) declareFixedRoles() error {
+	registration := accesscontrol.RoleRegistration{
+		Role: accesscontrol.RoleDTO{
+			Version:     1,
+			Name:        "fixed:provisioning:admin",
+			Description: "Reload provisioning configurations",
+			Permissions: []accesscontrol.Permission{
+				{
+					Action: ActionProvisioningReload,
+					Scope:  ScopeProvisionersAll,
+				},
+			},
+		},
+		Grants: []string{accesscontrol.RoleGrafanaAdmin},
+	}
+
+	return hs.AccessControl.DeclareFixedRoles(registration)
+}

pkg/server/server.go

@@ -39,7 +39,7 @@ import (
 	_ "github.com/grafana/grafana/pkg/services/login/loginservice"
 	_ "github.com/grafana/grafana/pkg/services/ngalert"
 	_ "github.com/grafana/grafana/pkg/services/notifications"
-	_ "github.com/grafana/grafana/pkg/services/provisioning"
+	"github.com/grafana/grafana/pkg/services/provisioning"
 	_ "github.com/grafana/grafana/pkg/services/rendering"
 	_ "github.com/grafana/grafana/pkg/services/search"
 	_ "github.com/grafana/grafana/pkg/services/sqlstore"
@@ -73,6 +73,11 @@ func (r *globalServiceRegistry) GetServices() []*registry.Descriptor {
 	return registry.GetServices()
 }
 
+type roleRegistry interface {
+	// RegisterFixedRoles registers all roles declared to AccessControl
+	RegisterFixedRoles() error
+}
+
 // New returns a new instance of Server.
 func New(cfg Config) (*Server, error) {
 	s := newServer(cfg)
@@ -130,7 +135,9 @@ type Server struct {
 
 	serviceRegistry serviceRegistry
 
-	HTTPServer *api.HTTPServer `inject:""`
+	HTTPServer          *api.HTTPServer                  `inject:""`
+	AccessControl       roleRegistry                     `inject:""`
+	ProvisioningService provisioning.ProvisioningService `inject:""`
 }
 
 // init initializes the server and its services.
@@ -167,7 +174,12 @@ func (s *Server) init() error {
 		}
 	}
 
-	return nil
+	// Register all fixed roles
+	if err := s.AccessControl.RegisterFixedRoles(); err != nil {
+		return err
+	}
+
+	return s.ProvisioningService.RunInitProvisioners()
 }
 
 // Run initializes and starts services. This will block until all services have

pkg/services/accesscontrol/accesscontrol.go

@@ -15,6 +15,10 @@ type AccessControl interface {
 
 	// Middleware checks if service disabled or not to switch to fallback authorization.
 	IsDisabled() bool
+
+	// DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their
+	// assignments to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
+	DeclareFixedRoles(...RoleRegistration) error
 }
 
 func HasAccess(ac AccessControl, c *models.ReqContext) func(fallback func(*models.ReqContext) bool, permission string, scopes ...string) bool {

pkg/services/accesscontrol/errors.go

@@ -0,0 +1,8 @@
+package accesscontrol
+
+import "errors"
+
+var (
+	ErrFixedRolePrefixMissing = errors.New("fixed role should be prefixed with '" + FixedRolePrefix + "'")
+	ErrInvalidBuiltinRole     = errors.New("built-in role is not valid")
+)

pkg/services/accesscontrol/models.go

@@ -4,6 +4,13 @@ import (
 	"time"
 )
 
+// RoleRegistration stores a role and its assignments to built-in roles
+// (Viewer, Editor, Admin, Grafana Admin)
+type RoleRegistration struct {
+	Role   RoleDTO
+	Grants []string
+}
+
 type Role struct {
 	Version     int64  `json:"version"`
 	UID         string `json:"uid"`
@@ -42,9 +49,6 @@ func (p RoleDTO) Role() Role {
 const (
 	// Permission actions
 
-	// Provisioning actions
-	ActionProvisioningReload = "provisioning:reload"
-
 	// Users actions
 	ActionUsersRead     = "users:read"
 	ActionUsersWrite    = "users:write"
@@ -94,15 +98,13 @@ const (
 	// Global Scopes
 	ScopeGlobalUsersAll = "global:users:*"
 
-	// Users scopes
+	// Users scope
-	ScopeUsersSelf = "users:self"
+	ScopeUsersAll = "users:*"
-	ScopeUsersAll  = "users:*"
 
 	// Settings scope
 	ScopeSettingsAll = "settings:**"
-
-	// Services Scopes
-	ScopeServicesAll = "service:*"
 )
 
 const RoleGrafanaAdmin = "Grafana Admin"
+
+const FixedRolePrefix = "fixed:"

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go

@@ -15,9 +15,10 @@ import (
 
 // OSSAccessControlService is the service implementing role based access control.
 type OSSAccessControlService struct {
-	Cfg        *setting.Cfg          `inject:""`
+	Cfg           *setting.Cfg          `inject:""`
-	UsageStats usagestats.UsageStats `inject:""`
+	UsageStats    usagestats.UsageStats `inject:""`
-	Log        log.Logger
+	Log           log.Logger
+	registrations accesscontrol.RegistrationList
 }
 
 // Init initializes the OSSAccessControlService.
@@ -69,11 +70,11 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user
 	for _, builtin := range builtinRoles {
 		if roleNames, ok := accesscontrol.FixedRoleGrants[builtin]; ok {
 			for _, name := range roleNames {
-				r, exists := accesscontrol.FixedRoles[name]
+				role, exists := accesscontrol.FixedRoles[name]
 				if !exists {
 					continue
 				}
-				for _, p := range r.Permissions {
+				for _, p := range role.Permissions {
 					permission := p
 					permissions = append(permissions, &permission)
 				}
@@ -95,3 +96,82 @@ func (ac *OSSAccessControlService) GetUserBuiltInRoles(user *models.SignedInUser
 
 	return roles
 }
+
+func (ac *OSSAccessControlService) saveFixedRole(role accesscontrol.RoleDTO) {
+	if storedRole, ok := accesscontrol.FixedRoles[role.Name]; ok {
+		// If a package wants to override another package's role, the version
+		// needs to be increased. Hence, we don't overwrite a role with a
+		// greater version.
+		if storedRole.Version >= role.Version {
+			log.Debugf("role %v has already been stored in a greater version, skipping registration", role.Name)
+			return
+		}
+	}
+	// Save role
+	accesscontrol.FixedRoles[role.Name] = role
+}
+
+func (ac *OSSAccessControlService) assignFixedRole(role accesscontrol.RoleDTO, builtInRoles []string) {
+	for _, builtInRole := range builtInRoles {
+		// Only record new assignments
+		alreadyAssigned := false
+		assignments, ok := accesscontrol.FixedRoleGrants[builtInRole]
+		if ok {
+			for _, assignedRole := range assignments {
+				if assignedRole == role.Name {
+					log.Debugf("role %v has already been assigned to %v", role.Name, builtInRole)
+					alreadyAssigned = true
+				}
+			}
+		}
+		if !alreadyAssigned {
+			assignments = append(assignments, role.Name)
+			accesscontrol.FixedRoleGrants[builtInRole] = assignments
+		}
+	}
+}
+
+// RegisterFixedRoles registers all declared roles in RAM
+func (ac *OSSAccessControlService) RegisterFixedRoles() error {
+	// If accesscontrol is disabled no need to register roles
+	if ac.IsDisabled() {
+		return nil
+	}
+	var err error
+	ac.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
+		ac.registerFixedRole(registration.Role, registration.Grants)
+		return true
+	})
+	return err
+}
+
+// RegisterFixedRole saves a fixed role and assigns it to built-in roles
+func (ac *OSSAccessControlService) registerFixedRole(role accesscontrol.RoleDTO, builtInRoles []string) {
+	ac.saveFixedRole(role)
+	ac.assignFixedRole(role, builtInRoles)
+}
+
+// DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their assignments
+// to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
+func (ac *OSSAccessControlService) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistration) error {
+	// If accesscontrol is disabled no need to register roles
+	if ac.IsDisabled() {
+		return nil
+	}
+
+	for _, r := range registrations {
+		err := accesscontrol.ValidateFixedRole(r.Role)
+		if err != nil {
+			return err
+		}
+
+		err = accesscontrol.ValidateBuiltInRoles(r.Grants)
+		if err != nil {
+			return err
+		}
+
+		ac.registrations.Append(r)
+	}
+
+	return nil
+}

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go

@@ -2,6 +2,7 @@ package ossaccesscontrol
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -31,6 +32,28 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService {
 	return &ac
 }
 
+func removeRoleHelper(role string) {
+	delete(accesscontrol.FixedRoles, role)
+
+	// Compute new grants removing any appearance of the role in the list
+	replaceGrants := map[string][]string{}
+
+	for builtInRole, grants := range accesscontrol.FixedRoleGrants {
+		newGrants := make([]string, len(grants))
+		for _, r := range grants {
+			if r != role {
+				newGrants = append(newGrants, r)
+			}
+		}
+		replaceGrants[builtInRole] = newGrants
+	}
+
+	// Replace grants
+	for br, grants := range replaceGrants {
+		accesscontrol.FixedRoleGrants[br] = grants
+	}
+}
+
 type usageStatsMock struct {
 	t            *testing.T
 	metricsFuncs []usagestats.MetricsFunc
@@ -166,3 +189,349 @@ func TestUsageMetrics(t *testing.T) {
 		})
 	}
 }
+
+type assignmentTestCase struct {
+	role         accesscontrol.RoleDTO
+	builtInRoles []string
+}
+
+func TestOSSAccessControlService_RegisterFixedRole(t *testing.T) {
+	tests := []struct {
+		name string
+		runs []assignmentTestCase
+	}{
+		{
+			name: "Successfully register role no assignments",
+			runs: []assignmentTestCase{
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+				},
+			},
+		},
+		{
+			name: "Successfully ignore overwriting existing role",
+			runs: []assignmentTestCase{
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+				},
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+				},
+			},
+		},
+		{
+			name: "Successfully register and assign role",
+			runs: []assignmentTestCase{
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					builtInRoles: []string{"Viewer", "Editor", "Admin"},
+				},
+			},
+		},
+		{
+			name: "Successfully ignore unchanged assignment",
+			runs: []assignmentTestCase{
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					builtInRoles: []string{"Viewer"},
+				},
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 2,
+						Name:    "fixed:test:test",
+					},
+					builtInRoles: []string{"Viewer"},
+				},
+			},
+		},
+		{
+			name: "Successfully add a new assignment",
+			runs: []assignmentTestCase{
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					builtInRoles: []string{"Viewer"},
+				},
+				{
+					role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					builtInRoles: []string{"Editor"},
+				},
+			},
+		},
+	}
+
+	// Check all runs performed so far to get the number of assignments seeder
+	// should have recorded
+	getTotalAssignCount := func(curRunIdx int, runs []assignmentTestCase) int {
+		builtIns := map[string]struct{}{}
+		for i := 0; i < curRunIdx+1; i++ {
+			for _, br := range runs[i].builtInRoles {
+				builtIns[br] = struct{}{}
+			}
+		}
+		return len(builtIns)
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			ac := &OSSAccessControlService{
+				Cfg:        setting.NewCfg(),
+				UsageStats: &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
+				Log:        log.New("accesscontrol-test"),
+			}
+
+			for i, run := range tc.runs {
+				// Remove any inserted role after the test case has been run
+				t.Cleanup(func() { removeRoleHelper(run.role.Name) })
+
+				ac.registerFixedRole(run.role, run.builtInRoles)
+
+				// Check role has been registered
+				storedRole, ok := accesscontrol.FixedRoles[run.role.Name]
+				assert.True(t, ok, "role should have been registered")
+
+				// Check registered role has not been altered
+				assert.Equal(t, run.role, storedRole, "role should not have been altered")
+
+				// Check assignments
+				// Count number of times the role has been assigned
+				assignCnt := 0
+				for _, grants := range accesscontrol.FixedRoleGrants {
+					for _, r := range grants {
+						if r == run.role.Name {
+							assignCnt++
+						}
+					}
+				}
+				assert.Equal(t, getTotalAssignCount(i, tc.runs), assignCnt,
+					"assignments should only be added, never removed")
+
+				for _, br := range run.builtInRoles {
+					assigns, ok := accesscontrol.FixedRoleGrants[br]
+					assert.True(t, ok,
+						fmt.Sprintf("role %s should have been assigned to %s", run.role.Name, br))
+					assert.Contains(t, assigns, run.role.Name,
+						fmt.Sprintf("role %s should have been assigned to %s", run.role.Name, br))
+				}
+			}
+		})
+	}
+}
+
+func TestOSSAccessControlService_DeclareFixedRoles(t *testing.T) {
+	tests := []struct {
+		name          string
+		registrations []accesscontrol.RoleRegistration
+		wantErr       bool
+		err           error
+	}{
+		{
+			name:    "should work with empty list",
+			wantErr: false,
+		},
+		{
+			name: "should add registration",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should fail registration invalid role name",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "custom:test:test",
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: true,
+			err:     accesscontrol.ErrFixedRolePrefixMissing,
+		},
+		{
+			name: "should fail registration invalid builtin role assignment",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					Grants: []string{"WrongAdmin"},
+				},
+			},
+			wantErr: true,
+			err:     accesscontrol.ErrInvalidBuiltinRole,
+		},
+		{
+			name: "should add multiple registrations at once",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					Grants: []string{"Admin"},
+				},
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test2:test2",
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ac := &OSSAccessControlService{
+				Cfg:           setting.NewCfg(),
+				UsageStats:    &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
+				Log:           log.New("accesscontrol-test"),
+				registrations: accesscontrol.RegistrationList{},
+			}
+			ac.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
+
+			// Test
+			err := ac.DeclareFixedRoles(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 TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {
+	tests := []struct {
+		name          string
+		token         models.Licensing
+		registrations []accesscontrol.RoleRegistration
+		wantErr       bool
+	}{
+		{
+			name: "should work with empty list",
+		},
+		{
+			name: "should register and assign role",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should register and assign multiple roles",
+			registrations: []accesscontrol.RoleRegistration{
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test:test",
+					},
+					Grants: []string{"Admin"},
+				},
+				{
+					Role: accesscontrol.RoleDTO{
+						Version: 1,
+						Name:    "fixed:test2:test2",
+					},
+					Grants: []string{"Admin"},
+				},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		cfg := setting.NewCfg()
+		cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
+
+		t.Run(tt.name, func(t *testing.T) {
+			// Remove any inserted role after the test case has been run
+			t.Cleanup(func() {
+				for _, registration := range tt.registrations {
+					removeRoleHelper(registration.Role.Name)
+				}
+			})
+
+			// Setup
+			ac := &OSSAccessControlService{
+				Cfg:           setting.NewCfg(),
+				UsageStats:    &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
+				Log:           log.New("accesscontrol-test"),
+				registrations: accesscontrol.RegistrationList{},
+			}
+			ac.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
+			ac.registrations.Append(tt.registrations...)
+
+			// Test
+			err := ac.RegisterFixedRoles()
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			// Check
+			for _, registration := range tt.registrations {
+				role, ok := accesscontrol.FixedRoles[registration.Role.Name]
+				assert.True(t, ok,
+					fmt.Sprintf("role %s should have been registered", registration.Role.Name))
+				assert.NotNil(t, role,
+					fmt.Sprintf("role %s should have been registered", registration.Role.Name))
+
+				for _, br := range registration.Grants {
+					rolesWithGrant, ok := accesscontrol.FixedRoleGrants[br]
+					assert.True(t, ok,
+						fmt.Sprintf("role %s should have been assigned to %s", registration.Role.Name, br))
+					assert.Contains(t, rolesWithGrant, registration.Role.Name,
+						fmt.Sprintf("role %s should have been assigned to %s", registration.Role.Name, br))
+				}
+			}
+		})
+	}
+}

pkg/services/accesscontrol/roles.go

@@ -1,198 +1,173 @@
 package accesscontrol
 
-import "github.com/grafana/grafana/pkg/models"
+import (
+	"fmt"
+	"strings"
+	"sync"
 
-var datasourcesEditorReadRole = RoleDTO{
+	"github.com/grafana/grafana/pkg/models"
-	Version: 1,
+)
-	Name:    datasourcesEditorRead,
-	Permissions: []Permission{
-		{
-			Action: ActionDatasourcesExplore,
-		},
-	},
-}
 
-var ldapAdminReadRole = RoleDTO{
+// Roles definition
-	Name:    ldapAdminRead,
+var (
-	Version: 1,
+	datasourcesEditorReadRole = RoleDTO{
-	Permissions: []Permission{
+		Version: 1,
-		{
+		Name:    datasourcesEditorRead,
-			Action: ActionLDAPUsersRead,
+		Permissions: []Permission{
+			{
+				Action: ActionDatasourcesExplore,
+			},
 		},
-		{
+	}
-			Action: ActionLDAPStatusRead,
-		},
-	},
-}
 
-var ldapAdminEditRole = RoleDTO{
+	ldapAdminReadRole = RoleDTO{
-	Name:    ldapAdminEdit,
+		Name:    ldapAdminRead,
-	Version: 2,
+		Version: 1,
-	Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{
+		Permissions: []Permission{
-		{
+			{
-			Action: ActionLDAPUsersSync,
+				Action: ActionLDAPUsersRead,
+			},
+			{
+				Action: ActionLDAPStatusRead,
+			},
 		},
-		{
+	}
-			Action: ActionLDAPConfigReload,
-		},
-	}),
-}
 
-var serverAdminReadRole = RoleDTO{
+	ldapAdminEditRole = RoleDTO{
-	Version: 1,
+		Name:    ldapAdminEdit,
-	Name:    serverAdminRead,
+		Version: 2,
-	Permissions: []Permission{
+		Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{
-		{
+			{
-			Action: ActionServerStatsRead,
+				Action: ActionLDAPUsersSync,
-		},
+			},
-	},
+			{
-}
+				Action: ActionLDAPConfigReload,
+			},
+		}),
+	}
 
-var settingsAdminReadRole = RoleDTO{
+	serverAdminReadRole = RoleDTO{
-	Version: 1,
+		Version: 1,
-	Name:    settingsAdminRead,
+		Name:    serverAdminRead,
-	Permissions: []Permission{
+		Permissions: []Permission{
-		{
+			{
-			Action: ActionSettingsRead,
+				Action: ActionServerStatsRead,
-			Scope:  ScopeSettingsAll,
+			},
 		},
-	},
+	}
-}
 
-var usersOrgReadRole = RoleDTO{
+	settingsAdminReadRole = RoleDTO{
-	Name:    usersOrgRead,
+		Version: 1,
-	Version: 1,
+		Name:    settingsAdminRead,
-	Permissions: []Permission{
+		Permissions: []Permission{
-		{
+			{
-			Action: ActionOrgUsersRead,
+				Action: ActionSettingsRead,
-			Scope:  ScopeUsersAll,
+				Scope:  ScopeSettingsAll,
+			},
 		},
-	},
+	}
-}
 
-var usersOrgEditRole = RoleDTO{
+	usersOrgReadRole = RoleDTO{
-	Name:    usersOrgEdit,
+		Name:    usersOrgRead,
-	Version: 1,
+		Version: 1,
-	Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{
+		Permissions: []Permission{
-		{
+			{
-			Action: ActionOrgUsersAdd,
+				Action: ActionOrgUsersRead,
-			Scope:  ScopeUsersAll,
+				Scope:  ScopeUsersAll,
+			},
 		},
-		{
+	}
-			Action: ActionOrgUsersRoleUpdate,
-			Scope:  ScopeUsersAll,
-		},
-		{
-			Action: ActionOrgUsersRemove,
-			Scope:  ScopeUsersAll,
-		},
-	}),
-}
 
-var usersAdminReadRole = RoleDTO{
+	usersOrgEditRole = RoleDTO{
-	Name:    usersAdminRead,
+		Name:    usersOrgEdit,
-	Version: 1,
+		Version: 1,
-	Permissions: []Permission{
+		Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{
-		{
+			{
-			Action: ActionUsersRead,
+				Action: ActionOrgUsersAdd,
-			Scope:  ScopeGlobalUsersAll,
+				Scope:  ScopeUsersAll,
-		},
+			},
-		{
+			{
-			Action: ActionUsersTeamRead,
+				Action: ActionOrgUsersRoleUpdate,
-			Scope:  ScopeGlobalUsersAll,
+				Scope:  ScopeUsersAll,
-		},
+			},
-		{
+			{
-			Action: ActionUsersAuthTokenList,
+				Action: ActionOrgUsersRemove,
-			Scope:  ScopeGlobalUsersAll,
+				Scope:  ScopeUsersAll,
-		},
+			},
-		{
+		}),
-			Action: ActionUsersQuotasList,
+	}
-			Scope:  ScopeGlobalUsersAll,
-		},
-	},
-}
 
-var usersAdminEditRole = RoleDTO{
+	usersAdminReadRole = RoleDTO{
-	Name:    usersAdminEdit,
+		Name:    usersAdminRead,
-	Version: 1,
+		Version: 1,
-	Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{
+		Permissions: []Permission{
-		{
+			{
-			Action: ActionUsersPasswordUpdate,
+				Action: ActionUsersRead,
-			Scope:  ScopeGlobalUsersAll,
+				Scope:  ScopeGlobalUsersAll,
+			},
+			{
+				Action: ActionUsersTeamRead,
+				Scope:  ScopeGlobalUsersAll,
+			},
+			{
+				Action: ActionUsersAuthTokenList,
+				Scope:  ScopeGlobalUsersAll,
+			},
+			{
+				Action: ActionUsersQuotasList,
+				Scope:  ScopeGlobalUsersAll,
+			},
 		},
-		{
+	}
-			Action: ActionUsersCreate,
-		},
-		{
-			Action: ActionUsersWrite,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersDelete,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersEnable,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersDisable,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersPermissionsUpdate,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersLogout,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersAuthTokenUpdate,
-			Scope:  ScopeGlobalUsersAll,
-		},
-		{
-			Action: ActionUsersQuotasUpdate,
-			Scope:  ScopeGlobalUsersAll,
-		},
-	}),
-}
 
-var provisioningAdminRole = RoleDTO{
+	usersAdminEditRole = RoleDTO{
-	Name:    provisioningAdmin,
+		Name:    usersAdminEdit,
-	Version: 1,
+		Version: 1,
-	Permissions: []Permission{
+		Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{
-		{
+			{
-			Action: ActionProvisioningReload,
+				Action: ActionUsersPasswordUpdate,
-			Scope:  ScopeServicesAll,
+				Scope:  ScopeGlobalUsersAll,
-		},
+			},
-	},
+			{
-}
+				Action: ActionUsersCreate,
-
+			},
-// FixedRoles provides a map of permission sets/roles which can be
+			{
-// assigned to a set of users. When adding a new resource protected by
+				Action: ActionUsersWrite,
-// Grafana access control the default permissions should be added to a
+				Scope:  ScopeGlobalUsersAll,
-// new fixed role in this set so that users can access the new
+			},
-// resource. FixedRoleGrants lists which built-in roles are
+			{
-// assigned which fixed roles in this list.
+				Action: ActionUsersDelete,
-var FixedRoles = map[string]RoleDTO{
+				Scope:  ScopeGlobalUsersAll,
-	datasourcesEditorRead: datasourcesEditorReadRole,
+			},
-	serverAdminRead:       serverAdminReadRole,
+			{
-
+				Action: ActionUsersEnable,
-	settingsAdminRead: settingsAdminReadRole,
+				Scope:  ScopeGlobalUsersAll,
-
+			},
-	usersAdminRead: usersAdminReadRole,
+			{
-	usersAdminEdit: usersAdminEditRole,
+				Action: ActionUsersDisable,
-
+				Scope:  ScopeGlobalUsersAll,
-	usersOrgRead: usersOrgReadRole,
+			},
-	usersOrgEdit: usersOrgEditRole,
+			{
-
+				Action: ActionUsersPermissionsUpdate,
-	ldapAdminRead: ldapAdminReadRole,
+				Scope:  ScopeGlobalUsersAll,
-	ldapAdminEdit: ldapAdminEditRole,
+			},
-
+			{
-	provisioningAdmin: provisioningAdminRole,
+				Action: ActionUsersLogout,
-}
+				Scope:  ScopeGlobalUsersAll,
+			},
+			{
+				Action: ActionUsersAuthTokenUpdate,
+				Scope:  ScopeGlobalUsersAll,
+			},
+			{
+				Action: ActionUsersQuotasUpdate,
+				Scope:  ScopeGlobalUsersAll,
+			},
+		}),
+	}
+)
 
+// Role names definitions
 const (
 	datasourcesEditorRead = "fixed:datasources:editor:read"
 
@@ -208,32 +183,49 @@ const (
 
 	ldapAdminEdit = "fixed:ldap:admin:edit"
 	ldapAdminRead = "fixed:ldap:admin:read"
-
-	provisioningAdmin = "fixed:provisioning:admin"
 )
 
-// FixedRoleGrants specifies which built-in roles are assigned
+var (
-// to which set of FixedRoles by default. Alphabetically sorted.
+	// FixedRoles provides a map of permission sets/roles which can be
-var FixedRoleGrants = map[string][]string{
+	// assigned to a set of users. When adding a new resource protected by
-	RoleGrafanaAdmin: {
+	// Grafana access control the default permissions should be added to a
-		ldapAdminEdit,
+	// new fixed role in this set so that users can access the new
-		ldapAdminRead,
+	// resource. FixedRoleGrants lists which built-in roles are
-		provisioningAdmin,
+	// assigned which fixed roles in this list.
-		serverAdminRead,
+	FixedRoles = map[string]RoleDTO{
-		settingsAdminRead,
+		datasourcesEditorRead: datasourcesEditorReadRole,
-		usersAdminEdit,
+		usersAdminEdit:        usersAdminEditRole,
-		usersAdminRead,
+		usersAdminRead:        usersAdminReadRole,
-		usersOrgEdit,
+		usersOrgEdit:          usersOrgEditRole,
-		usersOrgRead,
+		usersOrgRead:          usersOrgReadRole,
-	},
+		ldapAdminEdit:         ldapAdminEditRole,
-	string(models.ROLE_ADMIN): {
+		ldapAdminRead:         ldapAdminReadRole,
-		usersOrgEdit,
+		serverAdminRead:       serverAdminReadRole,
-		usersOrgRead,
+		settingsAdminRead:     settingsAdminReadRole,
-	},
+	}
-	string(models.ROLE_EDITOR): {
+
-		datasourcesEditorRead,
+	// FixedRoleGrants specifies which built-in roles are assigned
-	},
+	// to which set of FixedRoles by default. Alphabetically sorted.
-}
+	FixedRoleGrants = map[string][]string{
+		RoleGrafanaAdmin: {
+			ldapAdminEdit,
+			ldapAdminRead,
+			serverAdminRead,
+			settingsAdminRead,
+			usersAdminEdit,
+			usersAdminRead,
+			usersOrgEdit,
+			usersOrgRead,
+		},
+		string(models.ROLE_ADMIN): {
+			usersOrgEdit,
+			usersOrgRead,
+		},
+		string(models.ROLE_EDITOR): {
+			datasourcesEditorRead,
+		},
+	}
+)
 
 func ConcatPermissions(permissions ...[]Permission) []Permission {
 	if permissions == nil {
@@ -247,3 +239,42 @@ func ConcatPermissions(permissions ...[]Permission) []Permission {
 	}
 	return perms
 }
+
+// ValidateFixedRole errors when a fixed role does not match expected pattern
+func ValidateFixedRole(role RoleDTO) error {
+	if !strings.HasPrefix(role.Name, FixedRolePrefix) {
+		return ErrFixedRolePrefixMissing
+	}
+	return nil
+}
+
+// ValidateBuiltInRoles errors when a built-in role does not match expected pattern
+func ValidateBuiltInRoles(builtInRoles []string) error {
+	for _, br := range builtInRoles {
+		if !models.RoleType(br).IsValid() && br != RoleGrafanaAdmin {
+			return fmt.Errorf("'%s' %w", br, ErrInvalidBuiltinRole)
+		}
+	}
+	return nil
+}
+
+type RegistrationList struct {
+	mx            sync.RWMutex
+	registrations []RoleRegistration
+}
+
+func (m *RegistrationList) Append(regs ...RoleRegistration) {
+	m.mx.Lock()
+	defer m.mx.Unlock()
+	m.registrations = append(m.registrations, regs...)
+}
+
+func (m *RegistrationList) Range(f func(registration RoleRegistration) bool) {
+	m.mx.RLock()
+	defer m.mx.RUnlock()
+	for _, registration := range m.registrations {
+		if ok := f(registration); !ok {
+			return
+		}
+	}
+}

pkg/services/accesscontrol/roles_test.go

@@ -9,26 +9,28 @@ import (
 )
 
 func TestPredefinedRoles(t *testing.T) {
-	for name, r := range FixedRoles {
+	for name, role := range FixedRoles {
 		assert.Truef(t,
 			strings.HasPrefix(name, "fixed:"),
 			"expected all fixed roles to be prefixed by 'fixed:', found role '%s'", name,
 		)
-		assert.Equal(t, name, r.Name)
+		assert.Equal(t, name, role.Name)
-		assert.NotZero(t, r.Version)
+		assert.NotZero(t, role.Version)
-		// assert.NotEmpty(t, r.Description)
 	}
 }
 
 func TestPredefinedRoleGrants(t *testing.T) {
-	for _, v := range FixedRoleGrants {
+	for _, grants := range FixedRoleGrants {
+		// Check grants list is sorted
 		assert.True(t,
-			sort.SliceIsSorted(v, func(i, j int) bool {
+			sort.SliceIsSorted(grants, func(i, j int) bool {
-				return v[i] < v[j]
+				return grants[i] < grants[j]
 			}),
 			"require role grant lists to be sorted",
 		)
-		for _, r := range v {
+
+		// Check all granted roles have been registered
+		for _, r := range grants {
 			assert.Contains(t, FixedRoles, r)
 		}
 	}

pkg/services/provisioning/provisioning.go

@@ -78,7 +78,7 @@ type provisioningServiceImpl struct {
 }
 
 func (ps *provisioningServiceImpl) Init() error {
-	return ps.RunInitProvisioners()
+	return nil
 }
 
 func (ps *provisioningServiceImpl) RunInitProvisioners() error {