AccessControl: Remove package lists for roles and grants (#47141)

* AccessControl: Remove package variables for roles and grants Co-authored-by: Jguer <joao.guerreiro@grafana.com> * Check for inheritance during role registration Co-authored-by: Jguer <joao.guerreiro@grafana.com> * Moving back role definition to accessscontrol * Make settings reader role public Co-authored-by: Jguer <joao.guerreiro@grafana.com> * Nits Co-authored-by: Jguer <joao.guerreiro@grafana.com> * Forgot to update this * Account for declaration error * Fixing pkg/api init ossac * Account for error in tests * Update test to verify inheritance * Nits. * Place br inheritance behind a feature toggle * Parent -> Parents * Nit. Co-authored-by: Jguer <joao.guerreiro@grafana.com>
2025-02-25 18:55:37 -06:00 · 2022-04-06 09:31:14 +02:00 · 2022-04-06 09:31:14 +02:00 · f7305965a4
commit f7305965a4
parent 05b1edb4a7
6 changed files with 220 additions and 354 deletions
--- a/pkg/api/common_test.go
+++ b/pkg/api/common_test.go
@ -400,8 +400,9 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo
 		require.NoError(t, err)
 		hs.teamPermissionsService = teamPermissionService
 	} else {
-		ac := ossaccesscontrol.ProvideService(hs.Features, &usagestats.UsageStatsMock{T: t},
+		ac, errInitAc := ossaccesscontrol.ProvideService(hs.Features, &usagestats.UsageStatsMock{T: t},
 			database.ProvideService(db), routing.NewRouteRegister())
+		require.NoError(t, errInitAc)
 		hs.AccessControl = ac
 		// Perform role registration
 		err := hs.declareFixedRoles()
--- a/pkg/models/org_user.go
+++ b/pkg/models/org_user.go
@ -49,6 +49,17 @@ func (r RoleType) Children() []RoleType {
 	}
 }

+func (r RoleType) Parents() []RoleType {
+	switch r {
+	case ROLE_EDITOR:
+		return []RoleType{ROLE_ADMIN}
+	case ROLE_VIEWER:
+		return []RoleType{ROLE_EDITOR, ROLE_ADMIN}
+	default:
+		return nil
+	}
+}
+
 func (r *RoleType) UnmarshalJSON(data []byte) error {
 	var str string
 	err := json.Unmarshal(data, &str)
--- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go
+++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go
@ -16,38 +16,81 @@ import (
 )

 func ProvideService(features featuremgmt.FeatureToggles, usageStats usagestats.Service,
-	provider accesscontrol.PermissionsProvider, routeRegister routing.RouteRegister) *OSSAccessControlService {
-	s := ProvideOSSAccessControl(features, usageStats, provider)
-	s.registerUsageMetrics()
+	provider accesscontrol.PermissionsProvider, routeRegister routing.RouteRegister) (*OSSAccessControlService, error) {
+	var errDeclareRoles error
+	s := ProvideOSSAccessControl(features, provider)
+	s.registerUsageMetrics(usageStats)
 	if !s.IsDisabled() {
 		api := api.AccessControlAPI{
 			RouteRegister: routeRegister,
 			AccessControl: s,
 		}
 		api.RegisterAPIEndpoints()
+
+		errDeclareRoles = accesscontrol.DeclareFixedRoles(s)
+	}
+
+	return s, errDeclareRoles
+}
+
+func macroRoles() map[string]*accesscontrol.RoleDTO {
+	return map[string]*accesscontrol.RoleDTO{
+		string(models.ROLE_ADMIN): {
+			Name:        "fixed:builtins:admin",
+			DisplayName: string(models.ROLE_ADMIN),
+			Description: "Admin role",
+			Group:       "Basic",
+			Version:     1,
+			Permissions: []accesscontrol.Permission{},
+		},
+		string(models.ROLE_EDITOR): {
+			Name:        "fixed:builtins:editor",
+			DisplayName: string(models.ROLE_EDITOR),
+			Description: "Editor role",
+			Group:       "Basic",
+			Version:     1,
+			Permissions: []accesscontrol.Permission{},
+		},
+		string(models.ROLE_VIEWER): {
+			Name:        "fixed:builtins:viewer",
+			DisplayName: string(models.ROLE_VIEWER),
+			Description: "Viewer role",
+			Group:       "Basic",
+			Version:     1,
+			Permissions: []accesscontrol.Permission{},
+		},
+		accesscontrol.RoleGrafanaAdmin: {
+			Name:        "fixed:builtins:grafana_admin",
+			DisplayName: accesscontrol.RoleGrafanaAdmin,
+			Description: "Grafana Admin role",
+			Group:       "Basic",
+			Version:     1,
+			Permissions: []accesscontrol.Permission{},
+		},
 	}
-	return s
 }

 // ProvideOSSAccessControl creates an oss implementation of access control without usage stats registration
-func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, usageStats usagestats.Service, provider accesscontrol.PermissionsProvider) *OSSAccessControlService {
-	return &OSSAccessControlService{
+func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, provider accesscontrol.PermissionsProvider) *OSSAccessControlService {
+	s := &OSSAccessControlService{
 		features:      features,
 		provider:      provider,
-		usageStats:    usageStats,
 		log:           log.New("accesscontrol"),
 		scopeResolver: accesscontrol.NewScopeResolver(),
+		roles:         macroRoles(),
 	}
+
+	return s
 }

 // OSSAccessControlService is the service implementing role based access control.
 type OSSAccessControlService struct {
 	log           log.Logger
-	usageStats    usagestats.Service
 	features      featuremgmt.FeatureToggles
 	scopeResolver accesscontrol.ScopeResolver
 	provider      accesscontrol.PermissionsProvider
 	registrations accesscontrol.RegistrationList
+	roles         map[string]*accesscontrol.RoleDTO
 }

 func (ac *OSSAccessControlService) IsDisabled() bool {
@ -57,8 +100,8 @@ func (ac *OSSAccessControlService) IsDisabled() bool {
 	return !ac.features.IsEnabled(featuremgmt.FlagAccesscontrol)
 }

-func (ac *OSSAccessControlService) registerUsageMetrics() {
-	ac.usageStats.RegisterMetricsFunc(func(context.Context) (map[string]interface{}, error) {
+func (ac *OSSAccessControlService) registerUsageMetrics(usageStats usagestats.Service) {
+	usageStats.RegisterMetricsFunc(func(context.Context) (map[string]interface{}, error) {
 		return map[string]interface{}{
 			"stats.oss.accesscontrol.enabled.count": ac.getUsageMetrics(),
 		}, nil
@ -140,15 +183,9 @@ func (ac *OSSAccessControlService) getFixedPermissions(ctx context.Context, user
 	permissions := make([]*accesscontrol.Permission, 0)

 	for _, builtin := range ac.GetUserBuiltInRoles(user) {
-		if roleNames, ok := accesscontrol.FixedRoleGrants[builtin]; ok {
-			for _, name := range roleNames {
-				role, exists := accesscontrol.FixedRoles[name]
-				if !exists {
-					continue
-				}
-				for i := range role.Permissions {
-					permissions = append(permissions, &role.Permissions[i])
-				}
+		if macroRole, ok := ac.roles[builtin]; ok {
+			for i := range macroRole.Permissions {
+				permissions = append(permissions, &macroRole.Permissions[i])
 			}
 		}
 	}
@ -157,49 +194,20 @@ func (ac *OSSAccessControlService) getFixedPermissions(ctx context.Context, user
 }

 func (ac *OSSAccessControlService) GetUserBuiltInRoles(user *models.SignedInUser) []string {
-	roles := []string{string(user.OrgRole)}
-	for _, role := range user.OrgRole.Children() {
-		roles = append(roles, string(role))
+	builtInRoles := []string{string(user.OrgRole)}
+
+	// With built-in role simplifying, inheritance is performed upon role registration.
+	if !ac.features.IsEnabled(featuremgmt.FlagAccesscontrolBuiltins) {
+		for _, br := range user.OrgRole.Children() {
+			builtInRoles = append(builtInRoles, string(br))
+		}
 	}
+
 	if user.IsGrafanaAdmin {
-		roles = append(roles, accesscontrol.RoleGrafanaAdmin)
+		builtInRoles = append(builtInRoles, accesscontrol.RoleGrafanaAdmin)
 	}

-	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 {
-			ac.log.Debug("the role has already been stored in a greater version, skipping registration", "role", 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 {
-					ac.log.Debug("the role has already been assigned", "rolename", role.Name, "build_in_role", builtInRole)
-					alreadyAssigned = true
-				}
-			}
-		}
-		if !alreadyAssigned {
-			assignments = append(assignments, role.Name)
-			accesscontrol.FixedRoleGrants[builtInRole] = assignments
-		}
-	}
+	return builtInRoles
 }

 // RegisterFixedRoles registers all declared roles in RAM
@ -208,18 +216,33 @@ func (ac *OSSAccessControlService) RegisterFixedRoles() error {
 	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
+	return nil
 }

 // 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)
+	// Inheritance
+	brs := map[string]struct{}{}
+	for _, builtInRole := range builtInRoles {
+		brs[builtInRole] = struct{}{}
+		if builtInRole != accesscontrol.RoleGrafanaAdmin {
+			for _, parent := range models.RoleType(builtInRole).Parents() {
+				brs[string(parent)] = struct{}{}
+			}
+		}
+	}
+
+	for br := range brs {
+		if macroRole, ok := ac.roles[br]; ok {
+			macroRole.Permissions = append(macroRole.Permissions, role.Permissions...)
+		} else {
+			ac.log.Error("Unknown builtin role", "builtInRole", br)
+		}
+	}
 }

 // DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their assignments
--- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go
+++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go
@ -2,7 +2,6 @@ package ossaccesscontrol

 import (
 	"context"
-	"fmt"
 	"testing"

 	"github.com/stretchr/testify/assert"
@ -23,37 +22,16 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService {

 	ac := &OSSAccessControlService{
 		features:      featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol),
-		usageStats:    &usagestats.UsageStatsMock{T: t},
 		log:           log.New("accesscontrol"),
 		registrations: accesscontrol.RegistrationList{},
 		scopeResolver: accesscontrol.NewScopeResolver(),
 		provider:      database.ProvideService(sqlstore.InitTestDB(t)),
+		roles:         macroRoles(),
 	}
+	require.NoError(t, ac.RegisterFixedRoles())
 	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
-	}
-}
-
 // extractRawPermissionsHelper extracts action and scope fields only from a permission slice
 func extractRawPermissionsHelper(perms []*accesscontrol.Permission) []*accesscontrol.Permission {
 	res := make([]*accesscontrol.Permission, len(perms))
@ -112,6 +90,13 @@ func TestEvaluatingPermissions(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			ac := setupTestEnv(t)

+			// Use OSS roles for this test to pass
+			err := accesscontrol.DeclareFixedRoles(ac)
+			require.NoError(t, err)
+
+			errRegisterRoles := ac.RegisterFixedRoles()
+			require.NoError(t, errRegisterRoles)
+
 			user := &models.SignedInUser{
 				UserId:         1,
 				OrgId:          1,
@ -149,13 +134,16 @@ func TestUsageMetrics(t *testing.T) {

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := ProvideService(
+			usagestatsmock := &usagestats.UsageStatsMock{T: t}
+
+			_, errInitAc := ProvideService(
 				featuremgmt.WithFeatures("accesscontrol", tt.enabled),
-				&usagestats.UsageStatsMock{T: t},
+				usagestatsmock,
 				database.ProvideService(sqlstore.InitTestDB(t)),
 				routing.NewRouteRegister(),
 			)
-			report, err := s.usageStats.GetUsageReport(context.Background())
+			require.NoError(t, errInitAc)
+			report, err := usagestatsmock.GetUsageReport(context.Background())
 			assert.Nil(t, err)

 			assert.Equal(t, tt.expectedValue, report.Metrics["stats.oss.accesscontrol.enabled.count"])
@ -163,151 +151,35 @@ 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"},
-				},
-			},
-		},
+	perm := accesscontrol.Permission{Action: "test:test", Scope: "test:*"}
+
+	role := accesscontrol.RoleDTO{
+		Version:     1,
+		Name:        "fixed:test:test",
+		Permissions: []accesscontrol.Permission{perm},
+	}
+	builtInRoles := []string{"Editor"}
+
+	// Admin is going to get the role as well
+	includedBuiltInRoles := []string{"Editor", "Admin"}
+
+	// Grafana Admin and Viewer won't get the role
+	excludedbuiltInRoles := []string{"Viewer", "Grafana Admin"}
+
+	ac := setupTestEnv(t)
+	ac.registerFixedRole(role, builtInRoles)
+
+	for _, br := range includedBuiltInRoles {
+		builtinRole, ok := ac.roles[br]
+		assert.True(t, ok)
+		assert.Contains(t, builtinRole.Permissions, perm)
 	}

-	// 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{
-				features:   featuremgmt.WithFeatures(),
-				usageStats: &usagestats.UsageStatsMock{T: t},
-				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))
-				}
-			}
-		})
+	for _, br := range excludedbuiltInRoles {
+		builtinRole, ok := ac.roles[br]
+		assert.True(t, ok)
+		assert.NotContains(t, builtinRole.Permissions, perm)
 	}
 }

@ -388,6 +260,9 @@ func TestOSSAccessControlService_DeclareFixedRoles(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ac := setupTestEnv(t)

+			// Reset the registations
+			ac.registrations = accesscontrol.RegistrationList{}
+
 			// Test
 			err := ac.DeclareFixedRoles(tt.registrations...)
 			if tt.wantErr {
@ -423,10 +298,11 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {
 			registrations: []accesscontrol.RoleRegistration{
 				{
 					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
+						Version:     1,
+						Name:        "fixed:test:test",
+						Permissions: []accesscontrol.Permission{{Action: "test:test"}},
 					},
-					Grants: []string{"Admin"},
+					Grants: []string{"Editor"},
 				},
 			},
 			wantErr: false,
@ -436,17 +312,22 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {
 			registrations: []accesscontrol.RoleRegistration{
 				{
 					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
+						Version:     1,
+						Name:        "fixed:test:test",
+						Permissions: []accesscontrol.Permission{{Action: "test:test"}},
 					},
-					Grants: []string{"Admin"},
+					Grants: []string{"Editor"},
 				},
 				{
 					Role: accesscontrol.RoleDTO{
 						Version: 1,
 						Name:    "fixed:test2:test2",
+						Permissions: []accesscontrol.Permission{
+							{Action: "test:test2"},
+							{Action: "test:test3", Scope: "test:*"},
+						},
 					},
-					Grants: []string{"Admin"},
+					Grants: []string{"Viewer"},
 				},
 			},
 			wantErr: false,
@ -455,13 +336,8 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {

 	for _, tt := range tests {
 		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)
-				}
-			})
 			ac := setupTestEnv(t)
+
 			ac.registrations.Append(tt.registrations...)

 			// Test
@ -474,18 +350,24 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {

 			// 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))
-
+				// Prepare list of builtin roles to check
+				brAndParents := map[string]struct{}{}
 				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))
+					brAndParents[br] = struct{}{}
+					if br != accesscontrol.RoleGrafanaAdmin {
+						for _, parent := range models.RoleType(br).Parents() {
+							brAndParents[string(parent)] = struct{}{}
+						}
+					}
+				}
+
+				// Check builtin roles (parents included) have been granted with the permissions
+				for br := range brAndParents {
+					builtinRole, ok := ac.roles[br]
+					assert.True(t, ok)
+					for _, expectedPermission := range registration.Role.Permissions {
+						assert.Contains(t, builtinRole.Permissions, expectedPermission)
+					}
 				}
 			}
 		})
@ -529,11 +411,6 @@ func TestOSSAccessControlService_GetUserPermissions(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Remove any inserted role after the test case has been run
-			t.Cleanup(func() {
-				removeRoleHelper(registration.Role.Name)
-			})
-
 			// Setup
 			ac := setupTestEnv(t)

@ -614,11 +491,6 @@ func TestOSSAccessControlService_Evaluate(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Remove any inserted role after the test case has been run
-			t.Cleanup(func() {
-				removeRoleHelper(registration.Role.Name)
-			})
-
 			// Setup
 			ac := setupTestEnv(t)
 			ac.RegisterAttributeScopeResolver("users:login:", userLoginScopeSolver)
--- a/pkg/services/accesscontrol/roles.go
+++ b/pkg/services/accesscontrol/roles.go
@ -16,7 +16,7 @@ type RoleRegistry interface {
 // Roles definition
 var (
 	ldapReaderRole = RoleDTO{
-		Name:        ldapReader,
+		Name:        "fixed:ldap:reader",
 		DisplayName: "LDAP reader",
 		Description: "Read LDAP configuration and status.",
 		Group:       "LDAP",
@ -32,7 +32,7 @@ var (
 	}

 	ldapWriterRole = RoleDTO{
-		Name:        ldapWriter,
+		Name:        "fixed:ldap:writer",
 		DisplayName: "LDAP writer",
 		Description: "Read and update LDAP configuration and read LDAP status.",
 		Group:       "LDAP",
@ -48,7 +48,7 @@ var (
 	}

 	orgUsersWriterRole = RoleDTO{
-		Name:        orgUsersWriter,
+		Name:        "fixed:org.users:writer",
 		DisplayName: "Organization user writer",
 		Description: "Within a single organization, add a user, invite a user, read information about a user and their role, remove a user from that organization, or change the role of a user.",
 		Group:       "User administration (organizational)",
@ -70,7 +70,7 @@ var (
 	}

 	orgUsersReaderRole = RoleDTO{
-		Name:        orgUsersReader,
+		Name:        "fixed:org.users:reader",
 		DisplayName: "Organization user reader",
 		Description: "Read users within a single organization.",
 		Group:       "User administration (organizational)",
@ -83,12 +83,12 @@ var (
 		},
 	}

-	settingsReaderRole = RoleDTO{
-		Version:     4,
+	SettingsReaderRole = RoleDTO{
+		Name:        "fixed:settings:reader",
 		DisplayName: "Setting reader",
 		Description: "Read Grafana instance settings.",
 		Group:       "Settings",
-		Name:        settingsReader,
+		Version:     4,
 		Permissions: []Permission{
 			{
 				Action: ActionSettingsRead,
@ -98,11 +98,11 @@ var (
 	}

 	statsReaderRole = RoleDTO{
-		Version:     3,
-		Name:        statsReader,
+		Name:        "fixed:stats:reader",
 		DisplayName: "Statistics reader",
 		Description: "Read Grafana instance statistics.",
 		Group:       "Statistics",
+		Version:     3,
 		Permissions: []Permission{
 			{
 				Action: ActionServerStatsRead,
@ -111,7 +111,7 @@ var (
 	}

 	usersReaderRole = RoleDTO{
-		Name:        usersReader,
+		Name:        "fixed:users:reader",
 		DisplayName: "User reader",
 		Description: "Read all users and their information, such as team memberships, authentication tokens, and quotas.",
 		Group:       "User administration (global)",
@ -137,7 +137,7 @@ var (
 	}

 	usersWriterRole = RoleDTO{
-		Name:        usersWriter,
+		Name:        "fixed:users:writer",
 		DisplayName: "User writer",
 		Description: "Read and update all attributes and settings for all users in Grafana: update user information, read user information, create or enable or disable a user, make a user a Grafana administrator, sign out a user, update a user’s authentication token, or update quotas for all users.",
 		Group:       "User administration (global)",
@ -186,55 +186,44 @@ var (
 	}
 )

-// Role names definitions
-const (
-	ldapReader     = "fixed:ldap:reader"
-	ldapWriter     = "fixed:ldap:writer"
-	orgUsersReader = "fixed:org.users:reader"
-	orgUsersWriter = "fixed:org.users:writer"
-	settingsReader = "fixed:settings:reader"
-	statsReader    = "fixed:stats:reader"
-	usersReader    = "fixed:users:reader"
-	usersWriter    = "fixed:users:writer"
-)
-
-var (
-	// FixedRoles provides a map of permission sets/roles which can be
-	// assigned to a set of users. When adding a new resource protected by
-	// Grafana access control the default permissions should be added to a
-	// 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.
-	FixedRoles = map[string]RoleDTO{
-		ldapReader:     ldapReaderRole,
-		ldapWriter:     ldapWriterRole,
-		orgUsersReader: orgUsersReaderRole,
-		orgUsersWriter: orgUsersWriterRole,
-		settingsReader: settingsReaderRole,
-		statsReader:    statsReaderRole,
-		usersReader:    usersReaderRole,
-		usersWriter:    usersWriterRole,
+// Declare OSS roles to the accesscontrol service
+func DeclareFixedRoles(ac AccessControl) error {
+	ldapReader := RoleRegistration{
+		Role:   ldapReaderRole,
+		Grants: []string{RoleGrafanaAdmin},
+	}
+	ldapWriter := RoleRegistration{
+		Role:   ldapWriterRole,
+		Grants: []string{RoleGrafanaAdmin},
+	}
+	orgUsersReader := RoleRegistration{
+		Role:   orgUsersReaderRole,
+		Grants: []string{RoleGrafanaAdmin, string(models.ROLE_ADMIN)},
+	}
+	orgUsersWriter := RoleRegistration{
+		Role:   orgUsersWriterRole,
+		Grants: []string{RoleGrafanaAdmin, string(models.ROLE_ADMIN)},
+	}
+	settingsReader := RoleRegistration{
+		Role:   SettingsReaderRole,
+		Grants: []string{RoleGrafanaAdmin},
+	}
+	statsReader := RoleRegistration{
+		Role:   statsReaderRole,
+		Grants: []string{RoleGrafanaAdmin},
+	}
+	usersReader := RoleRegistration{
+		Role:   usersReaderRole,
+		Grants: []string{RoleGrafanaAdmin},
+	}
+	usersWriter := RoleRegistration{
+		Role:   usersWriterRole,
+		Grants: []string{RoleGrafanaAdmin},
 	}

-	// FixedRoleGrants specifies which built-in roles are assigned
-	// to which set of FixedRoles by default. Alphabetically sorted.
-	FixedRoleGrants = map[string][]string{
-		RoleGrafanaAdmin: {
-			ldapReader,
-			ldapWriter,
-			orgUsersReader,
-			orgUsersWriter,
-			settingsReader,
-			statsReader,
-			usersReader,
-			usersWriter,
-		},
-		string(models.ROLE_ADMIN): {
-			orgUsersReader,
-			orgUsersWriter,
-		},
-	}
-)
+	return ac.DeclareFixedRoles(ldapReader, ldapWriter, orgUsersReader, orgUsersWriter,
+		settingsReader, statsReader, usersReader, usersWriter)
+}

 func ConcatPermissions(permissions ...[]Permission) []Permission {
 	if permissions == nil {
--- a/pkg/services/accesscontrol/roles_test.go
+++ b/pkg/services/accesscontrol/roles_test.go
@ -1,41 +1,11 @@
 package accesscontrol

 import (
-	"sort"
-	"strings"
 	"testing"

 	"github.com/stretchr/testify/assert"
 )

-func TestFixedRoles(t *testing.T) {
-	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, role.Name)
-		assert.NotZero(t, role.Version)
-	}
-}
-
-func TestFixedRoleGrants(t *testing.T) {
-	for _, grants := range FixedRoleGrants {
-		// Check grants list is sorted
-		assert.True(t,
-			sort.SliceIsSorted(grants, func(i, j int) bool {
-				return grants[i] < grants[j]
-			}),
-			"require role grant lists to be sorted",
-		)
-
-		// Check all granted roles have been registered
-		for _, r := range grants {
-			assert.Contains(t, FixedRoles, r)
-		}
-	}
-}
-
 func TestConcatPermissions(t *testing.T) {
 	perms1 := []Permission{
 		{