Chore: remove `IsDisabled` method for access control (#74340)

remove IsDisabled method for access control, clean up tests

pkg/api/frontendsettings.go

@@ -120,7 +120,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro
 		VerifyEmailEnabled:                  setting.VerifyEmailEnabled,
 		SigV4AuthEnabled:                    setting.SigV4AuthEnabled,
 		AzureAuthEnabled:                    setting.AzureAuthEnabled,
-		RbacEnabled:                         hs.Cfg.RBACEnabled,
+		RbacEnabled:                         true,
 		ExploreEnabled:                      setting.ExploreEnabled,
 		HelpEnabled:                         setting.HelpEnabled,
 		ProfileEnabled:                      setting.ProfileEnabled,

pkg/api/frontendsettings_test.go

@@ -68,7 +68,7 @@ func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features *featuremgmt.
 		SettingsProvider:     setting.ProvideProvider(cfg),
 		pluginStore:          pluginStore,
 		grafanaUpdateChecker: &updatechecker.GrafanaService{},
-		AccessControl:        accesscontrolmock.New().WithDisabled(),
+		AccessControl:        accesscontrolmock.New(),
 		PluginSettings:       pluginsSettings,
 		pluginsCDNService: pluginscdn.ProvideService(&config.Cfg{
 			PluginsCDNURLTemplate: cfg.PluginsCDNURLTemplate,

pkg/api/plugins_test.go

@@ -65,7 +65,6 @@ func Test_PluginsInstallAndUninstall(t *testing.T) {
 	for _, tc := range tcs {
 		server := SetupAPITestServer(t, func(hs *HTTPServer) {
 			hs.Cfg = &setting.Cfg{
-				RBACEnabled:                      true,
 				PluginAdminEnabled:               tc.pluginAdminEnabled,
 				PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled}
 			hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}}

pkg/services/accesscontrol/accesscontrol.go

@@ -10,7 +10,6 @@ import (
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
-	"github.com/grafana/grafana/pkg/setting"
 )
 
 type AccessControl interface {
@@ -19,8 +18,6 @@ type AccessControl interface {
 	// RegisterScopeAttributeResolver allows the caller to register a scope resolver for a
 	// specific scope prefix (ex: datasources:name:)
 	RegisterScopeAttributeResolver(prefix string, resolver ScopeAttributeResolver)
-	//IsDisabled returns if access control is enabled or not
-	IsDisabled() bool
 }
 
 type Service interface {
@@ -43,8 +40,6 @@ type Service interface {
 	SaveExternalServiceRole(ctx context.Context, cmd SaveExternalServiceRoleCommand) error
 	// DeleteExternalServiceRole removes an external service's role and its assignment.
 	DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error
-	//IsDisabled returns if access control is enabled or not
-	IsDisabled() bool
 }
 
 type RoleRegistry interface {
@@ -371,10 +366,6 @@ func ManagedBuiltInRoleName(builtInRole string) string {
 	return fmt.Sprintf("managed:builtins:%s:permissions", strings.ToLower(builtInRole))
 }
 
-func IsDisabled(cfg *setting.Cfg) bool {
-	return !cfg.RBACEnabled
-}
-
 // GetOrgRoles returns legacy org roles for a user
 func GetOrgRoles(user identity.Requester) []string {
 	roles := []string{string(user.GetOrgRole())}

pkg/services/accesscontrol/acimpl/accesscontrol.go

@@ -63,7 +63,3 @@ func (a *AccessControl) Evaluate(ctx context.Context, user identity.Requester, e
 func (a *AccessControl) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) {
 	a.resolvers.AddScopeAttributeResolver(prefix, resolver)
 }
-
-func (a *AccessControl) IsDisabled() bool {
-	return accesscontrol.IsDisabled(a.cfg)
-}

pkg/services/accesscontrol/acimpl/service.go

@@ -215,10 +215,6 @@ func (s *Service) RegisterFixedRoles(ctx context.Context) error {
 	return nil
 }
 
-func (s *Service) IsDisabled() bool {
-	return accesscontrol.IsDisabled(s.cfg)
-}
-
 func permissionCacheKey(user identity.Requester) (string, error) {
 	key, err := user.GetCacheKey()
 	if err != nil {

pkg/services/accesscontrol/actest/fake.go

@@ -12,7 +12,6 @@ var _ accesscontrol.RoleRegistry = new(FakeService)
 
 type FakeService struct {
 	ExpectedErr                     error
-	ExpectedDisabled                bool
 	ExpectedCachedPermissions       bool
 	ExpectedPermissions             []accesscontrol.Permission
 	ExpectedFilteredUserPermissions []accesscontrol.Permission
@@ -49,10 +48,6 @@ func (f FakeService) RegisterFixedRoles(ctx context.Context) error {
 	return f.ExpectedErr
 }
 
-func (f FakeService) IsDisabled() bool {
-	return f.ExpectedDisabled
-}
-
 func (f FakeService) SaveExternalServiceRole(ctx context.Context, cmd accesscontrol.SaveExternalServiceRoleCommand) error {
 	return f.ExpectedErr
 }
@@ -65,7 +60,6 @@ var _ accesscontrol.AccessControl = new(FakeAccessControl)
 
 type FakeAccessControl struct {
 	ExpectedErr      error
-	ExpectedDisabled bool
 	ExpectedEvaluate bool
 }
 
@@ -76,10 +70,6 @@ func (f FakeAccessControl) Evaluate(ctx context.Context, user identity.Requester
 func (f FakeAccessControl) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) {
 }
 
-func (f FakeAccessControl) IsDisabled() bool {
-	return f.ExpectedDisabled
-}
-
 type FakeStore struct {
 	ExpectedUserPermissions  []accesscontrol.Permission
 	ExpectedUsersPermissions map[int64][]accesscontrol.Permission

pkg/services/accesscontrol/mock/mock.go

@@ -22,7 +22,6 @@ type Calls struct {
 	Evaluate                       []interface{}
 	GetUserPermissions             []interface{}
 	ClearUserPermissionCache       []interface{}
-	IsDisabled                     []interface{}
 	DeclareFixedRoles              []interface{}
 	DeclarePluginRoles             []interface{}
 	GetUserBuiltInRoles            []interface{}
@@ -38,8 +37,6 @@ type Calls struct {
 type Mock struct {
 	// Unless an override is provided, permissions will be returned by GetUserPermissions
 	permissions []accesscontrol.Permission
-	// Unless an override is provided, disabled will be returned by IsDisabled
-	disabled bool
 	// Unless an override is provided, builtInRoles will be returned by GetUserBuiltInRoles
 	builtInRoles []string
 
@@ -50,7 +47,6 @@ type Mock struct {
 	EvaluateFunc                       func(context.Context, *user.SignedInUser, accesscontrol.Evaluator) (bool, error)
 	GetUserPermissionsFunc             func(context.Context, *user.SignedInUser, accesscontrol.Options) ([]accesscontrol.Permission, error)
 	ClearUserPermissionCacheFunc       func(*user.SignedInUser)
-	IsDisabledFunc                     func() bool
 	DeclareFixedRolesFunc              func(...accesscontrol.RoleRegistration) error
 	DeclarePluginRolesFunc             func(context.Context, string, string, []plugins.RoleRegistration) error
 	GetUserBuiltInRolesFunc            func(user *user.SignedInUser) []string
@@ -72,7 +68,6 @@ var _ fullAccessControl = New()
 func New() *Mock {
 	mock := &Mock{
 		Calls:          Calls{},
-		disabled:       false,
 		permissions:    []accesscontrol.Permission{},
 		builtInRoles:   []string{},
 		scopeResolvers: accesscontrol.NewResolvers(log.NewNopLogger()),
@@ -90,11 +85,6 @@ func (m *Mock) WithPermissions(permissions []accesscontrol.Permission) *Mock {
 	return m
 }
 
-func (m *Mock) WithDisabled() *Mock {
-	m.disabled = true
-	return m
-}
-
 func (m *Mock) WithBuiltInRoles(builtInRoles []string) *Mock {
 	m.builtInRoles = builtInRoles
 	return m
@@ -160,18 +150,6 @@ func (m *Mock) ClearUserPermissionCache(usr identity.Requester) {
 	}
 }
 
-// Middleware checks if service disabled or not to switch to fallback authorization.
-// This mock return m.disabled unless an override is provided.
-func (m *Mock) IsDisabled() bool {
-	m.Calls.IsDisabled = append(m.Calls.IsDisabled, struct{}{})
-	// Use override if provided
-	if m.IsDisabledFunc != nil {
-		return m.IsDisabledFunc()
-	}
-	// Otherwise return the Disabled bool
-	return m.disabled
-}
-
 // DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their
 // assignments to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
 // This mock returns no error unless an override is provided.

pkg/services/ngalert/api/api_prometheus_test.go

@@ -502,7 +502,7 @@ func TestRouteGetRuleStatuses(t *testing.T) {
 				log:     log.NewNopLogger(),
 				manager: fakeAIM,
 				store:   ruleStore,
-				ac:      acmock.New().WithDisabled(),
+				ac:      acmock.New(),
 			}
 
 			response := api.RouteGetRuleStatuses(c)

pkg/services/ngalert/api/api_testing_test.go

@@ -271,7 +271,7 @@ func TestRouteEvalQueries(t *testing.T) {
 
 func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mock, evaluator eval.EvaluatorFactory) *TestingApiSrv {
 	if ac == nil {
-		ac = acMock.New().WithDisabled()
+		ac = acMock.New()
 	}
 
 	return &TestingApiSrv{

pkg/services/ngalert/api/util_test.go

@@ -60,7 +60,7 @@ func TestAlertingProxy_createProxyContext(t *testing.T) {
 
 	t.Run("should create a copy of request context", func(t *testing.T) {
 		for _, mock := range []*accesscontrolmock.Mock{
-			accesscontrolmock.New(), accesscontrolmock.New().WithDisabled(),
+			accesscontrolmock.New(), accesscontrolmock.New(),
 		} {
 			proxy := AlertingProxy{
 				DataProxy: nil,

pkg/services/quota/quotaimpl/quota_test.go

@@ -475,7 +475,7 @@ func setupEnv(t *testing.T, sqlStore *sqlstore.SQLStore, b bus.Bus, quotaService
 	require.NoError(t, err)
 	secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore())
 	secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger"))
-	_, err = dsservice.ProvideService(sqlStore, secretsService, secretsStore, sqlStore.Cfg, featuremgmt.WithFeatures(), acmock.New().WithDisabled(), acmock.NewMockedPermissionsService(), quotaService)
+	_, err = dsservice.ProvideService(sqlStore, secretsService, secretsStore, sqlStore.Cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService(), quotaService)
 	require.NoError(t, err)
 	m := metrics.NewNGAlert(prometheus.NewRegistry())
 	ruleStore, err := ngstore.ProvideDBStore(sqlStore.Cfg, featuremgmt.WithFeatures(), sqlStore, &foldertest.FakeService{},

pkg/services/secrets/kvstore/migrations/datasource_mig_test.go

@@ -30,7 +30,7 @@ func SetupTestDataSourceSecretMigrationService(t *testing.T, sqlStore db.DB, kvS
 	}
 	secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore())
 	quotaService := quotatest.New(false, nil)
-	dsService, err := dsservice.ProvideService(sqlStore, secretsService, secretsStore, cfg, features, acmock.New().WithDisabled(), acmock.NewMockedPermissionsService(), quotaService)
+	dsService, err := dsservice.ProvideService(sqlStore, secretsService, secretsStore, cfg, features, acmock.New(), acmock.NewMockedPermissionsService(), quotaService)
 	require.NoError(t, err)
 	migService := ProvideDataSourceMigrationService(dsService, kvStore, features)
 	return migService

pkg/setting/setting.go

@@ -536,7 +536,6 @@ type Cfg struct {
 	OAuth2ServerAccessTokenLifespan       time.Duration
 
 	// Access Control
-	RBACEnabled         bool
 	RBACPermissionCache bool
 	// Enable Permission validation during role creation and provisioning
 	RBACPermissionValidationEnabled bool
@@ -970,11 +969,10 @@ var skipStaticRootValidation = false
 
 func NewCfg() *Cfg {
 	return &Cfg{
-		Target:      []string{"all"},
-		Logger:      log.New("settings"),
-		Raw:         ini.Empty(),
-		Azure:       &azsettings.AzureSettings{},
-		RBACEnabled: true,
+		Target: []string{"all"},
+		Logger: log.New("settings"),
+		Raw:    ini.Empty(),
+		Azure:  &azsettings.AzureSettings{},
 	}
 }
 
@@ -1659,7 +1657,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
 
 func readAccessControlSettings(iniFile *ini.File, cfg *Cfg) {
 	rbac := iniFile.Section("rbac")
-	cfg.RBACEnabled = true
 	cfg.RBACPermissionCache = rbac.Key("permission_cache").MustBool(true)
 	cfg.RBACPermissionValidationEnabled = rbac.Key("permission_validation_enabled").MustBool(false)
 	cfg.RBACResetBasicRoles = rbac.Key("reset_basic_roles").MustBool(false)