From 55efeb0c02ef5261eb8a75ea27adfdc6194de7ad Mon Sep 17 00:00:00 2001 From: Emil Tullstedt Date: Fri, 30 Jul 2021 13:58:49 +0200 Subject: [PATCH] Revert "AccessControl: Implement a way to register fixed roles (#35641)" (#37397) This reverts commit 88c11f1cc0a7d0cfc99fafe4ffa64b54a78814e8. --- 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 | 441 ++++++++---------- pkg/services/accesscontrol/roles_test.go | 18 +- pkg/services/provisioning/provisioning.go | 2 +- 14 files changed, 237 insertions(+), 959 deletions(-) delete mode 100644 pkg/api/admin_provisioning_test.go delete mode 100644 pkg/api/roles.go delete mode 100644 pkg/services/accesscontrol/errors.go diff --git a/pkg/api/admin_provisioning_test.go b/pkg/api/admin_provisioning_test.go deleted file mode 100644 index da55d7ca05f..00000000000 --- a/pkg/api/admin_provisioning_test.go +++ /dev/null @@ -1,169 +0,0 @@ -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 ea64b095d52..94f1efc0359 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -445,11 +445,10 @@ 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", 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("/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("/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 db083c30e26..921f31532cb 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -253,10 +253,6 @@ 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 2fd0a608118..9f3f6b0f491 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -117,7 +117,8 @@ func (hs *HTTPServer) Init() error { hs.macaron = hs.newMacaron() hs.registerRoutes() - return hs.declareFixedRoles() + + return nil } func (hs *HTTPServer) AddMiddleware(middleware macaron.Handler) { diff --git a/pkg/api/roles.go b/pkg/api/roles.go deleted file mode 100644 index 12fc888ad6e..00000000000 --- a/pkg/api/roles.go +++ /dev/null @@ -1,41 +0,0 @@ -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 efe5d629623..ecc91158d73 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,11 +73,6 @@ 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) @@ -135,9 +130,7 @@ type Server struct { serviceRegistry serviceRegistry - HTTPServer *api.HTTPServer `inject:""` - AccessControl roleRegistry `inject:""` - ProvisioningService provisioning.ProvisioningService `inject:""` + HTTPServer *api.HTTPServer `inject:""` } // init initializes the server and its services. @@ -174,12 +167,7 @@ func (s *Server) init() error { } } - // Register all fixed roles - if err := s.AccessControl.RegisterFixedRoles(); err != nil { - return err - } - - return s.ProvisioningService.RunInitProvisioners() + return nil } // 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 9bd58ad413b..64e07b1b6c4 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -15,10 +15,6 @@ 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 deleted file mode 100644 index 593c67de1a2..00000000000 --- a/pkg/services/accesscontrol/errors.go +++ /dev/null @@ -1,8 +0,0 @@ -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 29f791a7bdd..2dea4614816 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -4,13 +4,6 @@ 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"` @@ -49,6 +42,9 @@ func (p RoleDTO) Role() Role { const ( // Permission actions + // Provisioning actions + ActionProvisioningReload = "provisioning:reload" + // Users actions ActionUsersRead = "users:read" ActionUsersWrite = "users:write" @@ -95,13 +91,15 @@ const ( // Global Scopes ScopeGlobalUsersAll = "global:users:*" - // Users scope - ScopeUsersAll = "users:*" + // Users scopes + ScopeUsersSelf = "users:self" + 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 a3919f5b77d..c635ab5c809 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go @@ -15,10 +15,9 @@ import ( // OSSAccessControlService is the service implementing role based access control. type OSSAccessControlService struct { - Cfg *setting.Cfg `inject:""` - UsageStats usagestats.UsageStats `inject:""` - Log log.Logger - registrations accesscontrol.RegistrationList + Cfg *setting.Cfg `inject:""` + UsageStats usagestats.UsageStats `inject:""` + Log log.Logger } // Init initializes the OSSAccessControlService. @@ -70,11 +69,11 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user for _, builtin := range builtinRoles { if roleNames, ok := accesscontrol.FixedRoleGrants[builtin]; ok { for _, name := range roleNames { - role, exists := accesscontrol.FixedRoles[name] + r, exists := accesscontrol.FixedRoles[name] if !exists { continue } - for _, p := range role.Permissions { + for _, p := range r.Permissions { permission := p permissions = append(permissions, &permission) } @@ -96,82 +95,3 @@ 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 91787e8d5dd..bb467afa456 100644 --- 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/grafana/grafana/pkg/infra/log" @@ -32,28 +31,6 @@ 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 @@ -189,349 +166,3 @@ 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 3823c98698d..104f4a72961 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -1,173 +1,198 @@ package accesscontrol -import ( - "fmt" - "strings" - "sync" +import "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/models" -) - -// Roles definition -var ( - datasourcesEditorReadRole = RoleDTO{ - Version: 1, - Name: datasourcesEditorRead, - Permissions: []Permission{ - { - Action: ActionDatasourcesExplore, - }, +var datasourcesEditorReadRole = RoleDTO{ + Version: 1, + Name: datasourcesEditorRead, + Permissions: []Permission{ + { + Action: ActionDatasourcesExplore, }, - } + }, +} - ldapAdminReadRole = RoleDTO{ - Name: ldapAdminRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionLDAPUsersRead, - }, - { - Action: ActionLDAPStatusRead, - }, +var ldapAdminReadRole = RoleDTO{ + Name: ldapAdminRead, + Version: 1, + Permissions: []Permission{ + { + Action: ActionLDAPUsersRead, }, - } - - ldapAdminEditRole = RoleDTO{ - Name: ldapAdminEdit, - Version: 2, - Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{ - { - Action: ActionLDAPUsersSync, - }, - { - Action: ActionLDAPConfigReload, - }, - }), - } - - serverAdminReadRole = RoleDTO{ - Version: 1, - Name: serverAdminRead, - Permissions: []Permission{ - { - Action: ActionServerStatsRead, - }, + { + Action: ActionLDAPStatusRead, }, - } + }, +} - settingsAdminReadRole = RoleDTO{ - Version: 1, - Name: settingsAdminRead, - Permissions: []Permission{ - { - Action: ActionSettingsRead, - Scope: ScopeSettingsAll, - }, +var ldapAdminEditRole = RoleDTO{ + Name: ldapAdminEdit, + Version: 2, + Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{ + { + Action: ActionLDAPUsersSync, }, - } - - usersOrgReadRole = RoleDTO{ - Name: usersOrgRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionOrgUsersRead, - Scope: ScopeUsersAll, - }, + { + Action: ActionLDAPConfigReload, }, - } + }), +} - usersOrgEditRole = RoleDTO{ - Name: usersOrgEdit, - Version: 1, - Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{ - { - Action: ActionOrgUsersAdd, - Scope: ScopeUsersAll, - }, - { - Action: ActionOrgUsersRoleUpdate, - Scope: ScopeUsersAll, - }, - { - Action: ActionOrgUsersRemove, - Scope: ScopeUsersAll, - }, - }), - } - - usersAdminReadRole = RoleDTO{ - Name: usersAdminRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionUsersRead, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersTeamRead, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersAuthTokenList, - Scope: ScopeGlobalUsersAll, - }, - { - Action: ActionUsersQuotasList, - Scope: ScopeGlobalUsersAll, - }, +var serverAdminReadRole = RoleDTO{ + Version: 1, + Name: serverAdminRead, + Permissions: []Permission{ + { + Action: ActionServerStatsRead, }, - } + }, +} - 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, - }, - }), - } -) +var settingsAdminReadRole = RoleDTO{ + Version: 1, + Name: settingsAdminRead, + Permissions: []Permission{ + { + Action: ActionSettingsRead, + Scope: ScopeSettingsAll, + }, + }, +} + +var usersOrgReadRole = RoleDTO{ + Name: usersOrgRead, + Version: 1, + Permissions: []Permission{ + { + Action: ActionOrgUsersRead, + Scope: ScopeUsersAll, + }, + }, +} + +var usersOrgEditRole = RoleDTO{ + Name: usersOrgEdit, + Version: 1, + Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{ + { + Action: ActionOrgUsersAdd, + 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, + }, + }, +} + +var 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, + }, + }), +} + +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, +} -// Role names definitions const ( datasourcesEditorRead = "fixed:datasources:editor:read" @@ -183,49 +208,32 @@ const ( ldapAdminEdit = "fixed:ldap:admin:edit" ldapAdminRead = "fixed:ldap:admin:read" + + provisioningAdmin = "fixed:provisioning:admin" ) -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, - }, - } -) +// 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, + }, +} func ConcatPermissions(permissions ...[]Permission) []Permission { if permissions == nil { @@ -239,42 +247,3 @@ 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 7df69ecd3f5..750abb43d63 100644 --- a/pkg/services/accesscontrol/roles_test.go +++ b/pkg/services/accesscontrol/roles_test.go @@ -9,28 +9,26 @@ import ( ) func TestPredefinedRoles(t *testing.T) { - for name, role := range FixedRoles { + for name, r := 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) + assert.Equal(t, name, r.Name) + assert.NotZero(t, r.Version) + // assert.NotEmpty(t, r.Description) } } func TestPredefinedRoleGrants(t *testing.T) { - for _, grants := range FixedRoleGrants { - // Check grants list is sorted + for _, v := range FixedRoleGrants { assert.True(t, - sort.SliceIsSorted(grants, func(i, j int) bool { - return grants[i] < grants[j] + sort.SliceIsSorted(v, func(i, j int) bool { + return v[i] < v[j] }), "require role grant lists to be sorted", ) - - // Check all granted roles have been registered - for _, r := range grants { + for _, r := range v { assert.Contains(t, FixedRoles, r) } } diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 6ec45b34389..b1ffc66ca17 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 nil + return ps.RunInitProvisioners() } func (ps *provisioningServiceImpl) RunInitProvisioners() error {