From e8e1a0b50ba8f6e4e2830a5e7912fb400508fe11 Mon Sep 17 00:00:00 2001 From: Jeremy Price Date: Wed, 4 Aug 2021 14:44:37 +0200 Subject: [PATCH] Revert "Revert "AccessControl: Implement a way to register fixed roles (#35641)" (#37397)" (#37535) This reverts commit 55efeb0c02ef5261eb8a75ea27adfdc6194de7ad. --- pkg/api/admin_provisioning_test.go | 169 +++++++ pkg/api/api.go | 9 +- pkg/api/common_test.go | 4 + pkg/api/http_server.go | 3 +- pkg/api/roles.go | 41 ++ pkg/server/server.go | 18 +- pkg/services/accesscontrol/accesscontrol.go | 4 + pkg/services/accesscontrol/errors.go | 8 + pkg/services/accesscontrol/models.go | 20 +- .../ossaccesscontrol/ossaccesscontrol.go | 90 +++- .../ossaccesscontrol/ossaccesscontrol_test.go | 369 +++++++++++++++ pkg/services/accesscontrol/roles.go | 431 ++++++++++-------- pkg/services/accesscontrol/roles_test.go | 18 +- pkg/services/provisioning/provisioning.go | 2 +- 14 files changed, 954 insertions(+), 232 deletions(-) create mode 100644 pkg/api/admin_provisioning_test.go create mode 100644 pkg/api/roles.go create mode 100644 pkg/services/accesscontrol/errors.go diff --git a/pkg/api/admin_provisioning_test.go b/pkg/api/admin_provisioning_test.go new file mode 100644 index 00000000000..da55d7ca05f --- /dev/null +++ b/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) + }) + } +} diff --git a/pkg/api/api.go b/pkg/api/api.go index 94c3c1af96d..9e8c55b66ad 100644 --- a/pkg/api/api.go +++ b/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/plugins/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadPlugins)) - adminRoute.Post("/provisioning/datasources/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadDatasources)) - adminRoute.Post("/provisioning/notifications/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadNotifications)) + adminRoute.Post("/provisioning/dashboards/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersDashboards), routing.Wrap(hs.AdminProvisioningReloadDashboards)) + adminRoute.Post("/provisioning/plugins/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersPlugins), routing.Wrap(hs.AdminProvisioningReloadPlugins)) + adminRoute.Post("/provisioning/datasources/reload", authorize(reqGrafanaAdmin, ActionProvisioningReload, ScopeProvisionersDatasources), routing.Wrap(hs.AdminProvisioningReloadDatasources)) + 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)) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 921f31532cb..db083c30e26 100644 --- a/pkg/api/common_test.go +++ b/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 diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 9f3f6b0f491..2fd0a608118 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -117,8 +117,7 @@ func (hs *HTTPServer) Init() error { hs.macaron = hs.newMacaron() hs.registerRoutes() - - return nil + return hs.declareFixedRoles() } func (hs *HTTPServer) AddMiddleware(middleware macaron.Handler) { diff --git a/pkg/api/roles.go b/pkg/api/roles.go new file mode 100644 index 00000000000..12fc888ad6e --- /dev/null +++ b/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) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index ecc91158d73..efe5d629623 100644 --- a/pkg/server/server.go +++ b/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 diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 64e07b1b6c4..9bd58ad413b 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/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 { diff --git a/pkg/services/accesscontrol/errors.go b/pkg/services/accesscontrol/errors.go new file mode 100644 index 00000000000..593c67de1a2 --- /dev/null +++ b/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") +) diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index e5e06d166a3..0fd557ebe15 100644 --- a/pkg/services/accesscontrol/models.go +++ b/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 - ScopeUsersSelf = "users:self" - ScopeUsersAll = "users:*" + // Users scope + ScopeUsersAll = "users:*" // Settings scope ScopeSettingsAll = "settings:**" - - // Services Scopes - ScopeServicesAll = "service:*" ) const RoleGrafanaAdmin = "Grafana Admin" + +const FixedRolePrefix = "fixed:" diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go index c635ab5c809..a3919f5b77d 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/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:""` - UsageStats usagestats.UsageStats `inject:""` - Log log.Logger + Cfg *setting.Cfg `inject:""` + UsageStats usagestats.UsageStats `inject:""` + 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 +} diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go index bb467afa456..91787e8d5dd 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go +++ b/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)) + } + } + }) + } +} diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 104f4a72961..3823c98698d 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/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{ - Version: 1, - Name: datasourcesEditorRead, - Permissions: []Permission{ - { - Action: ActionDatasourcesExplore, - }, - }, -} + "github.com/grafana/grafana/pkg/models" +) -var ldapAdminReadRole = RoleDTO{ - Name: ldapAdminRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionLDAPUsersRead, +// Roles definition +var ( + datasourcesEditorReadRole = RoleDTO{ + Version: 1, + Name: datasourcesEditorRead, + Permissions: []Permission{ + { + Action: ActionDatasourcesExplore, + }, }, - { - Action: ActionLDAPStatusRead, - }, - }, -} + } -var ldapAdminEditRole = RoleDTO{ - Name: ldapAdminEdit, - Version: 2, - Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{ - { - Action: ActionLDAPUsersSync, + ldapAdminReadRole = RoleDTO{ + Name: ldapAdminRead, + Version: 1, + Permissions: []Permission{ + { + Action: ActionLDAPUsersRead, + }, + { + Action: ActionLDAPStatusRead, + }, }, - { - Action: ActionLDAPConfigReload, - }, - }), -} + } -var serverAdminReadRole = RoleDTO{ - Version: 1, - Name: serverAdminRead, - Permissions: []Permission{ - { - Action: ActionServerStatsRead, - }, - }, -} + ldapAdminEditRole = RoleDTO{ + Name: ldapAdminEdit, + Version: 2, + Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{ + { + Action: ActionLDAPUsersSync, + }, + { + Action: ActionLDAPConfigReload, + }, + }), + } -var settingsAdminReadRole = RoleDTO{ - Version: 1, - Name: settingsAdminRead, - Permissions: []Permission{ - { - Action: ActionSettingsRead, - Scope: ScopeSettingsAll, + serverAdminReadRole = RoleDTO{ + Version: 1, + Name: serverAdminRead, + Permissions: []Permission{ + { + Action: ActionServerStatsRead, + }, }, - }, -} + } -var usersOrgReadRole = RoleDTO{ - Name: usersOrgRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionOrgUsersRead, - Scope: ScopeUsersAll, + settingsAdminReadRole = RoleDTO{ + Version: 1, + Name: settingsAdminRead, + Permissions: []Permission{ + { + Action: ActionSettingsRead, + Scope: ScopeSettingsAll, + }, }, - }, -} + } -var usersOrgEditRole = RoleDTO{ - Name: usersOrgEdit, - Version: 1, - Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{ - { - Action: ActionOrgUsersAdd, - Scope: ScopeUsersAll, + usersOrgReadRole = RoleDTO{ + Name: usersOrgRead, + Version: 1, + Permissions: []Permission{ + { + Action: ActionOrgUsersRead, + Scope: ScopeUsersAll, + }, }, - { - Action: ActionOrgUsersRoleUpdate, - Scope: ScopeUsersAll, - }, - { - Action: ActionOrgUsersRemove, - Scope: ScopeUsersAll, - }, - }), -} + } -var usersAdminReadRole = RoleDTO{ - Name: usersAdminRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionUsersRead, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersTeamRead, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersAuthTokenList, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersQuotasList, - Scope: ScopeGlobalUsersAll, - }, - }, -} + usersOrgEditRole = RoleDTO{ + Name: usersOrgEdit, + Version: 1, + Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{ + { + Action: ActionOrgUsersAdd, + Scope: ScopeUsersAll, + }, + { + Action: ActionOrgUsersRoleUpdate, + Scope: ScopeUsersAll, + }, + { + Action: ActionOrgUsersRemove, + Scope: ScopeUsersAll, + }, + }), + } -var usersAdminEditRole = RoleDTO{ - Name: usersAdminEdit, - Version: 1, - Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{ - { - Action: ActionUsersPasswordUpdate, - Scope: ScopeGlobalUsersAll, + usersAdminReadRole = RoleDTO{ + Name: usersAdminRead, + Version: 1, + Permissions: []Permission{ + { + Action: ActionUsersRead, + 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{ - Name: provisioningAdmin, - Version: 1, - Permissions: []Permission{ - { - Action: ActionProvisioningReload, - Scope: ScopeServicesAll, - }, - }, -} - -// 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. -var FixedRoles = map[string]RoleDTO{ - datasourcesEditorRead: datasourcesEditorReadRole, - serverAdminRead: serverAdminReadRole, - - settingsAdminRead: settingsAdminReadRole, - - usersAdminRead: usersAdminReadRole, - usersAdminEdit: usersAdminEditRole, - - usersOrgRead: usersOrgReadRole, - usersOrgEdit: usersOrgEditRole, - - ldapAdminRead: ldapAdminReadRole, - ldapAdminEdit: ldapAdminEditRole, - - provisioningAdmin: provisioningAdminRole, -} + usersAdminEditRole = RoleDTO{ + Name: usersAdminEdit, + Version: 1, + Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{ + { + Action: ActionUsersPasswordUpdate, + 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, + }, + }), + } +) +// 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 -// to which set of FixedRoles by default. Alphabetically sorted. -var FixedRoleGrants = map[string][]string{ - RoleGrafanaAdmin: { - ldapAdminEdit, - ldapAdminRead, - provisioningAdmin, - serverAdminRead, - settingsAdminRead, - usersAdminEdit, - usersAdminRead, - usersOrgEdit, - usersOrgRead, - }, - string(models.ROLE_ADMIN): { - usersOrgEdit, - usersOrgRead, - }, - string(models.ROLE_EDITOR): { - datasourcesEditorRead, - }, -} +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{ + datasourcesEditorRead: datasourcesEditorReadRole, + usersAdminEdit: usersAdminEditRole, + usersAdminRead: usersAdminReadRole, + usersOrgEdit: usersOrgEditRole, + usersOrgRead: usersOrgReadRole, + ldapAdminEdit: ldapAdminEditRole, + ldapAdminRead: ldapAdminReadRole, + serverAdminRead: serverAdminReadRole, + settingsAdminRead: settingsAdminReadRole, + } + + // 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 + } + } +} diff --git a/pkg/services/accesscontrol/roles_test.go b/pkg/services/accesscontrol/roles_test.go index 750abb43d63..7df69ecd3f5 100644 --- a/pkg/services/accesscontrol/roles_test.go +++ b/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.NotZero(t, r.Version) - // assert.NotEmpty(t, r.Description) + assert.Equal(t, name, role.Name) + assert.NotZero(t, role.Version) } } 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 { - return v[i] < v[j] + sort.SliceIsSorted(grants, func(i, j int) bool { + 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) } } diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index b1ffc66ca17..6ec45b34389 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/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 {