From 8bd825e16ca99baccd83d8325b02fe40c7640d43 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Tue, 12 Apr 2022 09:53:43 +0200 Subject: [PATCH] AccessControl: Make the built-in role definitions public (#47525) * AccessControl: Make the built-in role definitions public * Add context to RegisterFixedRoles Co-authored-by: Jguer * Making BuiltInRolesWithParents public to the AccessControl package Co-authored-by: Jguer Co-authored-by: Jguer --- pkg/api/common_test.go | 2 +- pkg/server/server.go | 2 +- pkg/services/accesscontrol/mock/mock.go | 4 +- pkg/services/accesscontrol/models.go | 20 ++++++- .../ossaccesscontrol/ossaccesscontrol.go | 54 ++----------------- .../ossaccesscontrol/ossaccesscontrol_test.go | 25 +++------ pkg/services/accesscontrol/roles.go | 48 ++++++++++++++++- 7 files changed, 79 insertions(+), 76 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 41ccf6ddd1c..c438c28a4c1 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -405,7 +405,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo // Perform role registration err := hs.declareFixedRoles() require.NoError(t, err) - err = ac.RegisterFixedRoles() + err = ac.RegisterFixedRoles(context.Background()) require.NoError(t, err) teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, ac, database.ProvideService(db)) require.NoError(t, err) diff --git a/pkg/server/server.go b/pkg/server/server.go index b1e8df32f94..d3c5ab9c88e 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -121,7 +121,7 @@ func (s *Server) init() error { login.ProvideService(s.HTTPServer.SQLStore, s.HTTPServer.Login) social.ProvideService(s.cfg) - if err := s.roleRegistry.RegisterFixedRoles(); err != nil { + if err := s.roleRegistry.RegisterFixedRoles(s.context); err != nil { return err } diff --git a/pkg/services/accesscontrol/mock/mock.go b/pkg/services/accesscontrol/mock/mock.go index 870ebcadbc9..c9278d4c9e7 100644 --- a/pkg/services/accesscontrol/mock/mock.go +++ b/pkg/services/accesscontrol/mock/mock.go @@ -10,7 +10,7 @@ import ( type fullAccessControl interface { accesscontrol.AccessControl GetUserBuiltInRoles(user *models.SignedInUser) []string - RegisterFixedRoles() error + RegisterFixedRoles(context.Context) error } type Calls struct { @@ -165,7 +165,7 @@ func (m *Mock) GetUserBuiltInRoles(user *models.SignedInUser) []string { // RegisterFixedRoles registers all roles declared to AccessControl // This mock returns no error unless an override is provided. -func (m *Mock) RegisterFixedRoles() error { +func (m *Mock) RegisterFixedRoles(ctx context.Context) error { m.Calls.RegisterFixedRoles = append(m.Calls.RegisterFixedRoles, []struct{}{}) // Use override if provided if m.RegisterFixedRolesFunc != nil { diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 6c5c3d576c3..11d856c4494 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -5,6 +5,7 @@ import ( "strings" "time" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/annotations" ) @@ -82,6 +83,7 @@ func (r RoleDTO) Role() Role { ID: r.ID, OrgID: r.OrgID, UID: r.UID, + Version: r.Version, Name: r.Name, DisplayName: r.DisplayName, Group: r.Group, @@ -244,6 +246,9 @@ type SetResourcePermissionCommand struct { const ( GlobalOrgID = 0 + FixedRolePrefix = "fixed:" + RoleGrafanaAdmin = "Grafana Admin" + GeneralFolderUID = "general" // Permission actions @@ -385,6 +390,17 @@ var ( ScopeAnnotationsTypeOrganization = ScopeAnnotationsProvider.GetResourceScopeType(annotations.Organization.String()) ) -const RoleGrafanaAdmin = "Grafana Admin" +func BuiltInRolesWithParents(builtInRoles []string) map[string]struct{} { + res := map[string]struct{}{} -const FixedRolePrefix = "fixed:" + for _, br := range builtInRoles { + res[br] = struct{}{} + if br != RoleGrafanaAdmin { + for _, parent := range models.RoleType(br).Parents() { + res[string(parent)] = struct{}{} + } + } + } + + return res +} diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go index 36ed825e626..15fe9dce61c 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go @@ -33,43 +33,6 @@ func ProvideService(features featuremgmt.FeatureToggles, usageStats usagestats.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{}, - }, - } -} - // ProvideOSSAccessControl creates an oss implementation of access control without usage stats registration func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, provider accesscontrol.PermissionsProvider) *OSSAccessControlService { s := &OSSAccessControlService{ @@ -77,7 +40,7 @@ func ProvideOSSAccessControl(features featuremgmt.FeatureToggles, provider acces provider: provider, log: log.New("accesscontrol"), scopeResolver: accesscontrol.NewScopeResolver(), - roles: macroRoles(), + roles: accesscontrol.BuildMacroRoleDefinitions(), } return s @@ -211,7 +174,7 @@ func (ac *OSSAccessControlService) GetUserBuiltInRoles(user *models.SignedInUser } // RegisterFixedRoles registers all declared roles in RAM -func (ac *OSSAccessControlService) RegisterFixedRoles() error { +func (ac *OSSAccessControlService) RegisterFixedRoles(ctx context.Context) error { // If accesscontrol is disabled no need to register roles if ac.IsDisabled() { return nil @@ -225,18 +188,7 @@ func (ac *OSSAccessControlService) RegisterFixedRoles() error { // RegisterFixedRole saves a fixed role and assigns it to built-in roles func (ac *OSSAccessControlService) registerFixedRole(role accesscontrol.RoleDTO, builtInRoles []string) { - // 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 { + for br := range accesscontrol.BuiltInRolesWithParents(builtInRoles) { if macroRole, ok := ac.roles[br]; ok { macroRole.Permissions = append(macroRole.Permissions, role.Permissions...) } else { diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go index f26fb33788c..df799ad1a3d 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go @@ -26,9 +26,9 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService { registrations: accesscontrol.RegistrationList{}, scopeResolver: accesscontrol.NewScopeResolver(), provider: database.ProvideService(sqlstore.InitTestDB(t)), - roles: macroRoles(), + roles: accesscontrol.BuildMacroRoleDefinitions(), } - require.NoError(t, ac.RegisterFixedRoles()) + require.NoError(t, ac.RegisterFixedRoles(context.Background())) return ac } @@ -94,7 +94,7 @@ func TestEvaluatingPermissions(t *testing.T) { err := accesscontrol.DeclareFixedRoles(ac) require.NoError(t, err) - errRegisterRoles := ac.RegisterFixedRoles() + errRegisterRoles := ac.RegisterFixedRoles(context.Background()) require.NoError(t, errRegisterRoles) user := &models.SignedInUser{ @@ -341,7 +341,7 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) { ac.registrations.Append(tt.registrations...) // Test - err := ac.RegisterFixedRoles() + err := ac.RegisterFixedRoles(context.Background()) if tt.wantErr { require.Error(t, err) return @@ -350,19 +350,8 @@ func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) { // Check for _, registration := range tt.registrations { - // Prepare list of builtin roles to check - brAndParents := map[string]struct{}{} - for _, br := range registration.Grants { - 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 { + for br := range accesscontrol.BuiltInRolesWithParents(registration.Grants) { builtinRole, ok := ac.roles[br] assert.True(t, ok) for _, expectedPermission := range registration.Role.Permissions { @@ -418,7 +407,7 @@ func TestOSSAccessControlService_GetUserPermissions(t *testing.T) { err := ac.DeclareFixedRoles(registration) require.NoError(t, err) - err = ac.RegisterFixedRoles() + err = ac.RegisterFixedRoles(context.Background()) require.NoError(t, err) // Test @@ -499,7 +488,7 @@ func TestOSSAccessControlService_Evaluate(t *testing.T) { err := ac.DeclareFixedRoles(registration) require.NoError(t, err) - err = ac.RegisterFixedRoles() + err = ac.RegisterFixedRoles(context.Background()) require.NoError(t, err) // Test diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 7561c836151..ac5082c3303 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -1,6 +1,7 @@ package accesscontrol import ( + "context" "fmt" "strings" "sync" @@ -10,7 +11,7 @@ import ( type RoleRegistry interface { // RegisterFixedRoles registers all roles declared to AccessControl - RegisterFixedRoles() error + RegisterFixedRoles(ctx context.Context) error } // Roles definition @@ -276,3 +277,48 @@ func (m *RegistrationList) Range(f func(registration RoleRegistration) bool) { } } } + +func BuildMacroRoleDefinitions() map[string]*RoleDTO { + return map[string]*RoleDTO{ + string(models.ROLE_ADMIN): { + Name: "grafana:builtins:admin", + UID: "grafana_builtins_admin", + OrgID: GlobalOrgID, + Version: 1, + DisplayName: string(models.ROLE_ADMIN), + Description: "Admin role", + Group: "Basic", + Permissions: []Permission{}, + }, + string(models.ROLE_EDITOR): { + Name: "grafana:builtins:editor", + UID: "grafana_builtins_editor", + OrgID: GlobalOrgID, + Version: 1, + DisplayName: string(models.ROLE_EDITOR), + Description: "Editor role", + Group: "Basic", + Permissions: []Permission{}, + }, + string(models.ROLE_VIEWER): { + Name: "grafana:builtins:viewer", + UID: "grafana_builtins_viewer", + OrgID: GlobalOrgID, + Version: 1, + DisplayName: string(models.ROLE_VIEWER), + Description: "Viewer role", + Group: "Basic", + Permissions: []Permission{}, + }, + RoleGrafanaAdmin: { + Name: "grafana:builtins:grafana_admin", + UID: "grafana_builtins_grafana_admin", + OrgID: GlobalOrgID, + Version: 1, + DisplayName: RoleGrafanaAdmin, + Description: "Grafana Admin role", + Group: "Basic", + Permissions: []Permission{}, + }, + } +}