AccessControl: Cleanup access control interface (#49783)

* removed unused function

* Rename interface

pkg/server/wireexts_oss.go

@@ -74,7 +74,7 @@ var wireExtsBasicSet = wire.NewSet(
 	wire.Bind(new(plugins.BackendFactoryProvider), new(*provider.Service)),
 	acdb.ProvideService,
 	wire.Bind(new(resourcepermissions.Store), new(*acdb.AccessControlStore)),
-	wire.Bind(new(accesscontrol.PermissionsProvider), new(*acdb.AccessControlStore)),
+	wire.Bind(new(accesscontrol.PermissionsStore), new(*acdb.AccessControlStore)),
 	osskmsproviders.ProvideService,
 	wire.Bind(new(kmsproviders.Service), new(osskmsproviders.Service)),
 	ldap.ProvideGroupsService,

pkg/services/accesscontrol/accesscontrol.go

@@ -23,9 +23,6 @@ type AccessControl interface {
 	// GetUserPermissions returns user permissions with only action and scope fields set.
 	GetUserPermissions(ctx context.Context, user *models.SignedInUser, options Options) ([]*Permission, error)
 
-	// GetUserRoles returns user roles.
-	GetUserRoles(ctx context.Context, user *models.SignedInUser) ([]*RoleDTO, error)
-
 	//IsDisabled returns if access control is enabled or not
 	IsDisabled() bool
 
@@ -38,7 +35,12 @@ type AccessControl interface {
 	RegisterScopeAttributeResolver(scopePrefix string, resolver ScopeAttributeResolver)
 }
 
-type PermissionsProvider interface {
+type RoleRegistry interface {
+	// RegisterFixedRoles registers all roles declared to AccessControl
+	RegisterFixedRoles(ctx context.Context) error
+}
+
+type PermissionsStore interface {
 	// GetUserPermissions returns user permissions with only action and scope fields set.
 	GetUserPermissions(ctx context.Context, query GetUserPermissionsQuery) ([]*Permission, error)
 }

pkg/services/accesscontrol/mock/mock.go

@@ -16,7 +16,6 @@ type fullAccessControl interface {
 type Calls struct {
 	Evaluate                       []interface{}
 	GetUserPermissions             []interface{}
-	GetUserRoles                   []interface{}
 	IsDisabled                     []interface{}
 	DeclareFixedRoles              []interface{}
 	GetUserBuiltInRoles            []interface{}
@@ -27,8 +26,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, roles will be returned by GetUserRoles
-	roles []*accesscontrol.RoleDTO
 	// Unless an override is provided, disabled will be returned by IsDisabled
 	disabled bool
 	// Unless an override is provided, builtInRoles will be returned by GetUserBuiltInRoles
@@ -40,7 +37,6 @@ type Mock struct {
 	// Override functions
 	EvaluateFunc                       func(context.Context, *models.SignedInUser, accesscontrol.Evaluator) (bool, error)
 	GetUserPermissionsFunc             func(context.Context, *models.SignedInUser, accesscontrol.Options) ([]*accesscontrol.Permission, error)
-	GetUserRolesFunc                   func(context.Context, *models.SignedInUser) ([]*accesscontrol.RoleDTO, error)
 	IsDisabledFunc                     func() bool
 	DeclareFixedRolesFunc              func(...accesscontrol.RoleRegistration) error
 	GetUserBuiltInRolesFunc            func(user *models.SignedInUser) []string
@@ -118,16 +114,6 @@ func (m *Mock) GetUserPermissions(ctx context.Context, user *models.SignedInUser
 	return m.permissions, nil
 }
 
-func (m *Mock) GetUserRoles(ctx context.Context, user *models.SignedInUser) ([]*accesscontrol.RoleDTO, error) {
-	m.Calls.GetUserRoles = append(m.Calls.GetUserRoles, []interface{}{ctx, user})
-	// Use override if provided
-	if m.GetUserRolesFunc != nil {
-		return m.GetUserRolesFunc(ctx, user)
-	}
-	// Otherwise return the Roles list
-	return m.roles, nil
-}
-
 // 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 {

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go

@@ -2,7 +2,6 @@ package ossaccesscontrol
 
 import (
 	"context"
-	"errors"
 
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -15,10 +14,12 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-func ProvideService(features featuremgmt.FeatureToggles, cfg *setting.Cfg,
-	provider accesscontrol.PermissionsProvider, routeRegister routing.RouteRegister) (*OSSAccessControlService, error) {
+func ProvideService(
+	features featuremgmt.FeatureToggles, cfg *setting.Cfg,
+	store accesscontrol.PermissionsStore, routeRegister routing.RouteRegister,
+) (*OSSAccessControlService, error) {
 	var errDeclareRoles error
-	s := ProvideOSSAccessControl(cfg, provider)
+	s := ProvideOSSAccessControl(cfg, store)
 	if !s.IsDisabled() {
 		api := api.AccessControlAPI{
 			RouteRegister: routeRegister,
@@ -32,10 +33,10 @@ func ProvideService(features featuremgmt.FeatureToggles, cfg *setting.Cfg,
 	return s, errDeclareRoles
 }
 
-func ProvideOSSAccessControl(cfg *setting.Cfg, provider accesscontrol.PermissionsProvider) *OSSAccessControlService {
+func ProvideOSSAccessControl(cfg *setting.Cfg, store accesscontrol.PermissionsStore) *OSSAccessControlService {
 	s := &OSSAccessControlService{
 		cfg:            cfg,
-		provider:       provider,
+		store:          store,
 		log:            log.New("accesscontrol"),
 		scopeResolvers: accesscontrol.NewScopeResolvers(),
 		roles:          accesscontrol.BuildBasicRoleDefinitions(),
@@ -49,7 +50,7 @@ type OSSAccessControlService struct {
 	log            log.Logger
 	cfg            *setting.Cfg
 	scopeResolvers accesscontrol.ScopeResolvers
-	provider       accesscontrol.PermissionsProvider
+	store          accesscontrol.PermissionsStore
 	registrations  accesscontrol.RegistrationList
 	roles          map[string]*accesscontrol.RoleDTO
 }
@@ -101,11 +102,6 @@ func (ac *OSSAccessControlService) Evaluate(ctx context.Context, user *models.Si
 	return resolvedEvaluator.Evaluate(user.Permissions[user.OrgId]), nil
 }
 
-// GetUserRoles returns user permissions based on built-in roles
-func (ac *OSSAccessControlService) GetUserRoles(ctx context.Context, user *models.SignedInUser) ([]*accesscontrol.RoleDTO, error) {
-	return nil, errors.New("unsupported function") //OSS users will continue to use builtin roles via GetUserPermissions
-}
-
 // GetUserPermissions returns user permissions based on built-in roles
 func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
 	timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
@@ -113,7 +109,7 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user
 
 	permissions := ac.getFixedPermissions(ctx, user)
 
-	dbPermissions, err := ac.provider.GetUserPermissions(ctx, accesscontrol.GetUserPermissionsQuery{
+	dbPermissions, err := ac.store.GetUserPermissions(ctx, accesscontrol.GetUserPermissionsQuery{
 		OrgID:   user.OrgId,
 		UserID:  user.UserId,
 		Roles:   ac.GetUserBuiltInRoles(user),

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go

@@ -27,7 +27,7 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService {
 		log:            log.New("accesscontrol"),
 		registrations:  accesscontrol.RegistrationList{},
 		scopeResolvers: accesscontrol.NewScopeResolvers(),
-		provider:       database.ProvideService(sqlstore.InitTestDB(t)),
+		store:          database.ProvideService(sqlstore.InitTestDB(t)),
 		roles:          accesscontrol.BuildBasicRoleDefinitions(),
 	}
 	require.NoError(t, ac.RegisterFixedRoles(context.Background()))

pkg/services/accesscontrol/roles.go

@@ -1,7 +1,6 @@
 package accesscontrol
 
 import (
-	"context"
 	"fmt"
 	"strings"
 	"sync"
@@ -9,11 +8,6 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 )
 
-type RoleRegistry interface {
-	// RegisterFixedRoles registers all roles declared to AccessControl
-	RegisterFixedRoles(ctx context.Context) error
-}
-
 // Roles definition
 var (
 	ldapReaderRole = RoleDTO{