From b827df626ddd45825771aff752fe1dc3a5584186 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 15 Sep 2022 11:34:15 +0200 Subject: [PATCH] RBAC: Initiate store in service (#55081) * RBAC: Dont inject store with wire * RBAC: Use Store interface * RBAC: Move store interface and initiate it from service --- pkg/api/common_test.go | 3 +-- pkg/cmd/grafana-cli/runner/wireexts_oss.go | 3 --- pkg/server/wireexts_oss.go | 3 --- pkg/services/accesscontrol/accesscontrol.go | 6 ------ pkg/services/accesscontrol/acimpl/service.go | 15 +++++++++++---- pkg/services/accesscontrol/acimpl/service_test.go | 2 +- pkg/services/accesscontrol/database/database.go | 6 +++--- 7 files changed, 16 insertions(+), 22 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index e64c9ae8116..c6399bd0da3 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -24,7 +24,6 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - "github.com/grafana/grafana/pkg/services/accesscontrol/database" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/auth" @@ -380,7 +379,7 @@ func setupHTTPServerWithCfgDb( acService = acmock } else { var err error - acService, err = acimpl.ProvideService(cfg, database.ProvideService(db), routeRegister, localcache.ProvideService()) + acService, err = acimpl.ProvideService(cfg, db, routeRegister, localcache.ProvideService()) require.NoError(t, err) ac = acimpl.ProvideAccessControl(cfg) } diff --git a/pkg/cmd/grafana-cli/runner/wireexts_oss.go b/pkg/cmd/grafana-cli/runner/wireexts_oss.go index 18fe5fb3798..407ce3c1945 100644 --- a/pkg/cmd/grafana-cli/runner/wireexts_oss.go +++ b/pkg/cmd/grafana-cli/runner/wireexts_oss.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/server/usagestatssvcs" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - acdb "github.com/grafana/grafana/pkg/services/accesscontrol/database" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/datasources" @@ -75,8 +74,6 @@ var wireExtsSet = wire.NewSet( wire.Bind(new(plugins.PluginLoaderAuthorizer), new(*signature.UnsignedPluginAuthorizer)), provider.ProvideService, wire.Bind(new(plugins.BackendFactoryProvider), new(*provider.Service)), - acdb.ProvideService, - wire.Bind(new(accesscontrol.Store), new(*acdb.AccessControlStore)), ldap.ProvideGroupsService, wire.Bind(new(ldap.Groups), new(*ldap.OSSGroups)), permissions.ProvideDatasourcePermissionsService, diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 4957b43d9d3..7dcf741694b 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/server/usagestatssvcs" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" - acdb "github.com/grafana/grafana/pkg/services/accesscontrol/database" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/datasources" @@ -74,8 +73,6 @@ var wireExtsBasicSet = wire.NewSet( wire.Bind(new(plugins.PluginLoaderAuthorizer), new(*signature.UnsignedPluginAuthorizer)), provider.ProvideService, wire.Bind(new(plugins.BackendFactoryProvider), new(*provider.Service)), - acdb.ProvideService, - wire.Bind(new(accesscontrol.Store), new(*acdb.AccessControlStore)), osskmsproviders.ProvideService, wire.Bind(new(kmsproviders.Service), new(osskmsproviders.Service)), ldap.ProvideGroupsService, diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 23114537555..d0cf1b1307a 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -45,12 +45,6 @@ type Options struct { ReloadCache bool } -type Store interface { - // GetUserPermissions returns user permissions with only action and scope fields set. - GetUserPermissions(ctx context.Context, query GetUserPermissionsQuery) ([]Permission, error) - DeleteUserPermissions(ctx context.Context, orgID, userID int64) error -} - type TeamPermissionsService interface { GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]ResourcePermission, error) SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error) diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index cd4aaf72a20..196df90a546 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -11,7 +11,9 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/api" + "github.com/grafana/grafana/pkg/services/accesscontrol/database" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/prometheus/client_golang/prometheus" @@ -21,8 +23,8 @@ const ( cacheTTL = 10 * time.Second ) -func ProvideService(cfg *setting.Cfg, store accesscontrol.Store, routeRegister routing.RouteRegister, cache *localcache.CacheService) (*Service, error) { - service := ProvideOSSService(cfg, store, cache) +func ProvideService(cfg *setting.Cfg, store sqlstore.Store, routeRegister routing.RouteRegister, cache *localcache.CacheService) (*Service, error) { + service := ProvideOSSService(cfg, database.ProvideService(store), cache) if !accesscontrol.IsDisabled(cfg) { api.NewAccessControlAPI(routeRegister, service).RegisterAPIEndpoints() @@ -34,7 +36,7 @@ func ProvideService(cfg *setting.Cfg, store accesscontrol.Store, routeRegister r return service, nil } -func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, cache *localcache.CacheService) *Service { +func ProvideOSSService(cfg *setting.Cfg, store store, cache *localcache.CacheService) *Service { s := &Service{ cfg: cfg, store: store, @@ -46,11 +48,16 @@ func ProvideOSSService(cfg *setting.Cfg, store accesscontrol.Store, cache *local return s } +type store interface { + GetUserPermissions(ctx context.Context, query accesscontrol.GetUserPermissionsQuery) ([]accesscontrol.Permission, error) + DeleteUserPermissions(ctx context.Context, orgID, userID int64) error +} + // Service is the service implementing role based access control. type Service struct { log log.Logger cfg *setting.Cfg - store accesscontrol.Store + store store cache *localcache.CacheService registrations accesscontrol.RegistrationList roles map[string]*accesscontrol.RoleDTO diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index d0dcc50b9c6..24d475e26d4 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -58,7 +58,7 @@ func TestUsageMetrics(t *testing.T) { s, errInitAc := ProvideService( cfg, - database.ProvideService(sqlstore.InitTestDB(t)), + sqlstore.InitTestDB(t), routing.NewRouteRegister(), localcache.ProvideService(), ) diff --git a/pkg/services/accesscontrol/database/database.go b/pkg/services/accesscontrol/database/database.go index 7bbef6b3085..7856232ed33 100644 --- a/pkg/services/accesscontrol/database/database.go +++ b/pkg/services/accesscontrol/database/database.go @@ -13,12 +13,12 @@ const ( globalOrgID = 0 ) -func ProvideService(sqlStore *sqlstore.SQLStore) *AccessControlStore { - return &AccessControlStore{sqlStore} +func ProvideService(sql sqlstore.Store) *AccessControlStore { + return &AccessControlStore{sql} } type AccessControlStore struct { - sql *sqlstore.SQLStore + sql sqlstore.Store } func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query accesscontrol.GetUserPermissionsQuery) ([]accesscontrol.Permission, error) {