diff --git a/pkg/registry/backgroundsvcs/background_services.go b/pkg/registry/backgroundsvcs/background_services.go index dd5b693a0b9..478abe93cc5 100644 --- a/pkg/registry/backgroundsvcs/background_services.go +++ b/pkg/registry/backgroundsvcs/background_services.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/cleanup" "github.com/grafana/grafana/pkg/services/dashboardsnapshots" + extsvcreg "github.com/grafana/grafana/pkg/services/extsvcauth/registry" grafanaapiserver "github.com/grafana/grafana/pkg/services/grafana-apiserver" "github.com/grafana/grafana/pkg/services/grpcserver" "github.com/grafana/grafana/pkg/services/guardian" @@ -58,7 +59,7 @@ func ProvideBackgroundServiceRegistry( bundleService *supportbundlesimpl.Service, publicDashboardsMetric *publicdashboardsmetric.Service, keyRetriever *dynamic.KeyRetriever, dynamicAngularDetectorsProvider *angulardetectorsprovider.Dynamic, grafanaAPIServer grafanaapiserver.Service, - anon *anonimpl.AnonDeviceService, + anon *anonimpl.AnonDeviceService, reg *extsvcreg.Registry, // Need to make sure these are initialized, is there a better place to put them? _ dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, @@ -100,6 +101,7 @@ func ProvideBackgroundServiceRegistry( dynamicAngularDetectorsProvider, grafanaAPIServer, anon, + reg, ) } diff --git a/pkg/services/extsvcauth/models.go b/pkg/services/extsvcauth/models.go index c1f5765e7a6..717eb42f67e 100644 --- a/pkg/services/extsvcauth/models.go +++ b/pkg/services/extsvcauth/models.go @@ -16,10 +16,14 @@ const ( type AuthProvider string +//go:generate mockery --name ExternalServiceRegistry --structname ExternalServiceRegistryMock --output tests --outpkg tests --filename extsvcregmock.go type ExternalServiceRegistry interface { // HasExternalService returns whether an external service has been saved with that name. HasExternalService(ctx context.Context, name string) (bool, error) + // GetExternalServiceNames returns the names of external services registered in store. + GetExternalServiceNames(ctx context.Context) ([]string, error) + // RemoveExternalService removes an external service and its associated resources from the database (ex: service account, token). RemoveExternalService(ctx context.Context, name string) error diff --git a/pkg/services/extsvcauth/oauthserver/models.go b/pkg/services/extsvcauth/oauthserver/models.go index a5923802170..70ba02ed31d 100644 --- a/pkg/services/extsvcauth/oauthserver/models.go +++ b/pkg/services/extsvcauth/oauthserver/models.go @@ -49,6 +49,7 @@ type OAuth2Server interface { type Store interface { DeleteExternalService(ctx context.Context, id string) error GetExternalService(ctx context.Context, id string) (*OAuthExternalService, error) + GetExternalServiceNames(ctx context.Context) ([]string, error) GetExternalServiceByName(ctx context.Context, name string) (*OAuthExternalService, error) GetExternalServicePublicKey(ctx context.Context, clientID string) (*jose.JSONWebKey, error) RegisterExternalService(ctx context.Context, client *OAuthExternalService) error diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go index 24f6b4366e1..a6f99863a27 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go @@ -193,6 +193,17 @@ func (s *OAuth2ServiceImpl) setClientUser(ctx context.Context, client *oauthserv return nil } +// GetExternalServiceNames get the names of External Service in store +func (s *OAuth2ServiceImpl) GetExternalServiceNames(ctx context.Context) ([]string, error) { + s.logger.Debug("Get external service names from store") + res, err := s.sqlstore.GetExternalServiceNames(ctx) + if err != nil { + s.logger.Error("Could not fetch clients from store", "error", err.Error()) + return nil, err + } + return res, nil +} + func (s *OAuth2ServiceImpl) RemoveExternalService(ctx context.Context, name string) error { s.logger.Info("Remove external service", "service", name) @@ -228,19 +239,20 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio s.logger.Warn("RegisterExternalService called without registration") return nil, nil } - s.logger.Info("Registering external service", "external service name", registration.Name) + slug := registration.Name + s.logger.Info("Registering external service", "external service", slug) // Check if the client already exists in store - client, errFetchExtSvc := s.sqlstore.GetExternalServiceByName(ctx, registration.Name) + client, errFetchExtSvc := s.sqlstore.GetExternalServiceByName(ctx, slug) if errFetchExtSvc != nil && !errors.Is(errFetchExtSvc, oauthserver.ErrClientNotFound) { - s.logger.Error("Error fetching service", "external service", registration.Name, "error", errFetchExtSvc) + s.logger.Error("Error fetching service", "external service", slug, "error", errFetchExtSvc) return nil, errFetchExtSvc } // Otherwise, create a new client if client == nil { - s.logger.Debug("External service does not yet exist", "external service name", registration.Name) + s.logger.Debug("External service does not yet exist", "external service", slug) client = &oauthserver.OAuthExternalService{ - Name: registration.Name, + Name: slug, ServiceAccountID: oauthserver.NoServiceAccountID, Audiences: s.cfg.AppURL, } diff --git a/pkg/services/extsvcauth/oauthserver/oastest/fakes.go b/pkg/services/extsvcauth/oauthserver/oastest/fakes.go index f492364b075..35a80fdab35 100644 --- a/pkg/services/extsvcauth/oauthserver/oastest/fakes.go +++ b/pkg/services/extsvcauth/oauthserver/oastest/fakes.go @@ -25,6 +25,10 @@ func (s *FakeService) GetExternalService(ctx context.Context, id string) (*oauth return s.ExpectedClient, s.ExpectedErr } +func (s *FakeService) GetExternalServiceNames(ctx context.Context) ([]string, error) { + return nil, nil +} + func (s *FakeService) RemoveExternalService(ctx context.Context, name string) error { return s.ExpectedErr } diff --git a/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go b/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go index 84c52f5372a..68d6aeab5c4 100644 --- a/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go +++ b/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go @@ -82,6 +82,32 @@ func (_m *MockStore) GetExternalServiceByName(ctx context.Context, name string) return r0, r1 } +// GetExternalServiceNames provides a mock function with given fields: ctx +func (_m *MockStore) GetExternalServiceNames(ctx context.Context) ([]string, error) { + ret := _m.Called(ctx) + + var r0 []string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) ([]string, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) []string); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetExternalServicePublicKey provides a mock function with given fields: ctx, clientID func (_m *MockStore) GetExternalServicePublicKey(ctx context.Context, clientID string) (*jose.JSONWebKey, error) { ret := _m.Called(ctx, clientID) diff --git a/pkg/services/extsvcauth/oauthserver/store/database.go b/pkg/services/extsvcauth/oauthserver/store/database.go index 3007b06afa6..bd0978b0980 100644 --- a/pkg/services/extsvcauth/oauthserver/store/database.go +++ b/pkg/services/extsvcauth/oauthserver/store/database.go @@ -213,6 +213,17 @@ func getExternalServiceByName(sess *db.Session, name string) (*oauthserver.OAuth return res, errPerm } +// FIXME: If we ever do a search method remove that method +func (s *store) GetExternalServiceNames(ctx context.Context) ([]string, error) { + res := []string{} + + err := s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + return sess.SQL(`SELECT name FROM oauth_client`).Find(&res) + }) + + return res, err +} + func (s *store) UpdateExternalServiceGrantTypes(ctx context.Context, clientID, grantTypes string) error { if clientID == "" { return oauthserver.ErrClientRequiredID diff --git a/pkg/services/extsvcauth/oauthserver/store/database_test.go b/pkg/services/extsvcauth/oauthserver/store/database_test.go index cd8f7078260..69b50ade317 100644 --- a/pkg/services/extsvcauth/oauthserver/store/database_test.go +++ b/pkg/services/extsvcauth/oauthserver/store/database_test.go @@ -435,6 +435,32 @@ func TestStore_RemoveExternalService(t *testing.T) { } } +func Test_store_GetExternalServiceNames(t *testing.T) { + ctx := context.Background() + client1 := oauthserver.OAuthExternalService{ + Name: "my-external-service", + ClientID: "ClientID", + ImpersonatePermissions: []accesscontrol.Permission{}, + } + client2 := oauthserver.OAuthExternalService{ + Name: "my-external-service-2", + ClientID: "ClientID2", + ImpersonatePermissions: []accesscontrol.Permission{ + {Action: "dashboards:read", Scope: "folders:*"}, + {Action: "dashboards:read", Scope: "dashboards:*"}, + }, + } + + // Init store + s := &store{db: db.InitTestDB(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagExternalServiceAuth}})} + require.NoError(t, s.SaveExternalService(context.Background(), &client1)) + require.NoError(t, s.SaveExternalService(context.Background(), &client2)) + + got, err := s.GetExternalServiceNames(ctx) + require.NoError(t, err) + require.ElementsMatch(t, []string{"my-external-service", "my-external-service-2"}, got) +} + func compareClientToStored(t *testing.T, s *store, wanted *oauthserver.OAuthExternalService) { ctx := context.Background() stored, err := s.GetExternalService(ctx, wanted.ClientID) diff --git a/pkg/services/extsvcauth/registry/service.go b/pkg/services/extsvcauth/registry/service.go index 204e93ec3c9..e5370e4df3a 100644 --- a/pkg/services/extsvcauth/registry/service.go +++ b/pkg/services/extsvcauth/registry/service.go @@ -35,12 +35,51 @@ func ProvideExtSvcRegistry(oauthServer *oasimpl.OAuth2ServiceImpl, saSvc *extsvc } } +// CleanUpOrphanedExternalServices remove external services present in store that have not been registered on startup. +func (r *Registry) CleanUpOrphanedExternalServices(ctx context.Context) error { + extsvcs, err := r.retrieveExtSvcProviders(ctx) + if err != nil { + r.logger.Error("Could not retrieve external services from store", "error", err.Error()) + return err + } + for name, provider := range extsvcs { + // The service did not register this time. Removed. + if _, ok := r.extSvcProviders[slugify.Slugify(name)]; !ok { + r.logger.Info("Detected removed External Service", "service", name, "provider", provider) + switch provider { + case extsvcauth.ServiceAccounts: + if err := r.saReg.RemoveExternalService(ctx, name); err != nil { + return err + } + case extsvcauth.OAuth2Server: + if err := r.oauthReg.RemoveExternalService(ctx, name); err != nil { + return err + } + } + } + } + return nil +} + // HasExternalService returns whether an external service has been saved with that name. func (r *Registry) HasExternalService(ctx context.Context, name string) (bool, error) { _, ok := r.extSvcProviders[slugify.Slugify(name)] return ok, nil } +// GetExternalServiceNames returns the list of external services registered in store. +func (r *Registry) GetExternalServiceNames(ctx context.Context) ([]string, error) { + extSvcProviders, err := r.retrieveExtSvcProviders(ctx) + if err != nil { + return nil, err + } + names := []string{} + for s := range extSvcProviders { + names = append(names, s) + } + return names, nil +} + // RemoveExternalService removes an external service and its associated resources from the database (ex: service account, token). func (r *Registry) RemoveExternalService(ctx context.Context, name string) error { provider, ok := r.extSvcProviders[slugify.Slugify(name)] @@ -97,3 +136,34 @@ func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.Exte return nil, extsvcauth.ErrUnknownProvider.Errorf("unknow provider '%v'", cmd.AuthProvider) } } + +// retrieveExtSvcProviders fetches external services from store and map their associated provider +func (r *Registry) retrieveExtSvcProviders(ctx context.Context) (map[string]extsvcauth.AuthProvider, error) { + extsvcs := map[string]extsvcauth.AuthProvider{} + if r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) { + names, err := r.saReg.GetExternalServiceNames(ctx) + if err != nil { + return nil, err + } + for i := range names { + extsvcs[names[i]] = extsvcauth.ServiceAccounts + } + } + // Important to run this second as the OAuth server uses External Service Accounts as well. + if r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAuth) { + names, err := r.oauthReg.GetExternalServiceNames(ctx) + if err != nil { + return nil, err + } + for i := range names { + extsvcs[names[i]] = extsvcauth.OAuth2Server + } + } + return extsvcs, nil +} + +func (r *Registry) Run(ctx context.Context) error { + // This is a one-time background job. + // Cleans up external services that have not been registered this time. + return r.CleanUpOrphanedExternalServices(ctx) +} diff --git a/pkg/services/extsvcauth/registry/service_test.go b/pkg/services/extsvcauth/registry/service_test.go new file mode 100644 index 00000000000..81912190551 --- /dev/null +++ b/pkg/services/extsvcauth/registry/service_test.go @@ -0,0 +1,122 @@ +package registry + +import ( + "context" + "sync" + "testing" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/extsvcauth/tests" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type TestEnv struct { + r *Registry + oauthReg *tests.ExternalServiceRegistryMock + saReg *tests.ExternalServiceRegistryMock +} + +func setupTestEnv(t *testing.T) *TestEnv { + env := TestEnv{} + env.oauthReg = tests.NewExternalServiceRegistryMock(t) + env.saReg = tests.NewExternalServiceRegistryMock(t) + env.r = &Registry{ + features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth, featuremgmt.FlagExternalServiceAccounts), + logger: log.New("extsvcauth.registry.test"), + oauthReg: env.oauthReg, + saReg: env.saReg, + extSvcProviders: map[string]extsvcauth.AuthProvider{}, + lock: sync.Mutex{}, + } + return &env +} + +func TestRegistry_CleanUpOrphanedExternalServices(t *testing.T) { + tests := []struct { + name string + init func(*TestEnv) + }{ + { + name: "should not clean up when every service registered", + init: func(te *TestEnv) { + // Have registered two services one requested a service account, the other requested to be an oauth client + te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server} + + te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil) + // Also return the external service account attached to the OAuth Server + te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "oauth-svc"}, nil) + }, + }, + { + name: "should clean up an orphaned service account", + init: func(te *TestEnv) { + // Have registered two services one requested a service account, the other requested to be an oauth client + te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server} + + te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil) + // Also return the external service account attached to the OAuth Server + te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "orphaned-sa-svc", "oauth-svc"}, nil) + + te.saReg.On("RemoveExternalService", mock.Anything, "orphaned-sa-svc").Return(nil) + }, + }, + { + name: "should clean up an orphaned OAuth Client", + init: func(te *TestEnv) { + // Have registered two services one requested a service account, the other requested to be an oauth client + te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server} + + te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc", "orphaned-oauth-svc"}, nil) + // Also return the external service account attached to the OAuth Server + te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "orphaned-oauth-svc", "oauth-svc"}, nil) + + te.oauthReg.On("RemoveExternalService", mock.Anything, "orphaned-oauth-svc").Return(nil) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := setupTestEnv(t) + tt.init(env) + + err := env.r.CleanUpOrphanedExternalServices(context.Background()) + require.NoError(t, err) + + env.oauthReg.AssertExpectations(t) + env.saReg.AssertExpectations(t) + }) + } +} + +func TestRegistry_GetExternalServiceNames(t *testing.T) { + tests := []struct { + name string + init func(*TestEnv) + want []string + }{ + { + name: "should deduplicate names", + init: func(te *TestEnv) { + te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "oauth-svc"}, nil) + te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil) + }, + want: []string{"sa-svc", "oauth-svc"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := setupTestEnv(t) + tt.init(env) + + names, err := env.r.GetExternalServiceNames(context.Background()) + require.NoError(t, err) + require.EqualValues(t, tt.want, names) + + env.oauthReg.AssertExpectations(t) + env.saReg.AssertExpectations(t) + }) + } +} diff --git a/pkg/services/extsvcauth/tests/extsvcregmock.go b/pkg/services/extsvcauth/tests/extsvcregmock.go new file mode 100644 index 00000000000..82b0a278108 --- /dev/null +++ b/pkg/services/extsvcauth/tests/extsvcregmock.go @@ -0,0 +1,119 @@ +// Code generated by mockery v2.35.2. DO NOT EDIT. + +package tests + +import ( + context "context" + + extsvcauth "github.com/grafana/grafana/pkg/services/extsvcauth" + mock "github.com/stretchr/testify/mock" +) + +// ExternalServiceRegistryMock is an autogenerated mock type for the ExternalServiceRegistry type +type ExternalServiceRegistryMock struct { + mock.Mock +} + +// GetExternalServiceNames provides a mock function with given fields: ctx +func (_m *ExternalServiceRegistryMock) GetExternalServiceNames(ctx context.Context) ([]string, error) { + ret := _m.Called(ctx) + + var r0 []string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) ([]string, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) []string); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// HasExternalService provides a mock function with given fields: ctx, name +func (_m *ExternalServiceRegistryMock) HasExternalService(ctx context.Context, name string) (bool, error) { + ret := _m.Called(ctx, name) + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { + return rf(ctx, name) + } + if rf, ok := ret.Get(0).(func(context.Context, string) bool); ok { + r0 = rf(ctx, name) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, name) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// RemoveExternalService provides a mock function with given fields: ctx, name +func (_m *ExternalServiceRegistryMock) RemoveExternalService(ctx context.Context, name string) error { + ret := _m.Called(ctx, name) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, name) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SaveExternalService provides a mock function with given fields: ctx, cmd +func (_m *ExternalServiceRegistryMock) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { + ret := _m.Called(ctx, cmd) + + var r0 *extsvcauth.ExternalService + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error)); ok { + return rf(ctx, cmd) + } + if rf, ok := ret.Get(0).(func(context.Context, *extsvcauth.ExternalServiceRegistration) *extsvcauth.ExternalService); ok { + r0 = rf(ctx, cmd) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*extsvcauth.ExternalService) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *extsvcauth.ExternalServiceRegistration) error); ok { + r1 = rf(ctx, cmd) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewExternalServiceRegistryMock creates a new instance of ExternalServiceRegistryMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewExternalServiceRegistryMock(t interface { + mock.TestingT + Cleanup(func()) +}) *ExternalServiceRegistryMock { + mock := &ExternalServiceRegistryMock{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/services/serviceaccounts/extsvcaccounts/metrics.go b/pkg/services/serviceaccounts/extsvcaccounts/metrics.go index b0da11e64f1..0e2942a50fd 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/metrics.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/metrics.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/serviceaccounts" - "github.com/grafana/grafana/pkg/services/user" "github.com/prometheus/client_golang/prometheus" ) @@ -30,15 +29,10 @@ func newMetrics(reg prometheus.Registerer, saSvc serviceaccounts.Service, logger ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() res, err := saSvc.SearchOrgServiceAccounts(ctx, &serviceaccounts.SearchOrgServiceAccountsQuery{ - OrgID: extsvcauth.TmpOrgID, - Filter: serviceaccounts.FilterOnlyExternal, - CountOnly: true, - SignedInUser: &user.SignedInUser{ - OrgID: extsvcauth.TmpOrgID, - Permissions: map[int64]map[string][]string{ - extsvcauth.TmpOrgID: {serviceaccounts.ActionRead: {"serviceaccounts:id:*"}}, - }, - }, + OrgID: extsvcauth.TmpOrgID, + Filter: serviceaccounts.FilterOnlyExternal, + CountOnly: true, + SignedInUser: extsvcuser, }) if err != nil { logger.Error("Could not compute extsvc_total metric", "error", err) diff --git a/pkg/services/serviceaccounts/extsvcaccounts/models.go b/pkg/services/serviceaccounts/extsvcaccounts/models.go index e10df1bd261..f6ab4ee4636 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/models.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/models.go @@ -3,6 +3,9 @@ package extsvcaccounts import ( "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/util/errutil" ) @@ -22,6 +25,13 @@ var ( ErrCannotListTokens = errutil.BadRequest("extsvcaccounts.ErrCannotListTokens", errutil.WithPublicMessage("cannot list external service account tokens")) ErrCredentialsNotFound = errutil.NotFound("extsvcaccounts.credentials-not-found") ErrInvalidName = errutil.BadRequest("extsvcaccounts.ErrInvalidName", errutil.WithPublicMessage("only external service account names can be prefixed with 'extsvc-'")) + + extsvcuser = &user.SignedInUser{ + OrgID: extsvcauth.TmpOrgID, + Permissions: map[int64]map[string][]string{ + extsvcauth.TmpOrgID: {serviceaccounts.ActionRead: {"serviceaccounts:id:*"}}, + }, + } ) // Credentials represents the credentials associated to an external service diff --git a/pkg/services/serviceaccounts/extsvcaccounts/service.go b/pkg/services/serviceaccounts/extsvcaccounts/service.go index dd0fcb09683..a8184736d12 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/service.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/service.go @@ -3,6 +3,7 @@ package extsvcaccounts import ( "context" "errors" + "strings" "github.com/prometheus/client_golang/prometheus" @@ -92,6 +93,28 @@ func (esa *ExtSvcAccountsService) RetrieveExtSvcAccount(ctx context.Context, org }, nil } +// GetExternalServiceNames get the names of External Service in store +func (esa *ExtSvcAccountsService) GetExternalServiceNames(ctx context.Context) ([]string, error) { + esa.logger.Debug("Get external service names from store") + sas, err := esa.saSvc.SearchOrgServiceAccounts(ctx, &sa.SearchOrgServiceAccountsQuery{ + OrgID: extsvcauth.TmpOrgID, + Filter: sa.FilterOnlyExternal, + SignedInUser: extsvcuser, + }) + if err != nil { + esa.logger.Error("Could not fetch external service accounts from store", "error", err.Error()) + return nil, err + } + if sas == nil { + return []string{}, nil + } + res := make([]string, len(sas.ServiceAccounts)) + for i := range sas.ServiceAccounts { + res[i] = strings.TrimPrefix(sas.ServiceAccounts[i].Name, sa.ExtSvcPrefix) + } + return res, nil +} + // SaveExternalService creates, updates or delete a service account (and its token) with the requested permissions. func (esa *ExtSvcAccountsService) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { // This is double proofing, we should never reach here anyway the flags have already been checked. @@ -135,7 +158,7 @@ func (esa *ExtSvcAccountsService) SaveExternalService(ctx context.Context, cmd * "error", err.Error()) return nil, err } - return &extsvcauth.ExternalService{Name: cmd.Name, ID: slug, Secret: token}, nil + return &extsvcauth.ExternalService{Name: slug, ID: slug, Secret: token}, nil } func (esa *ExtSvcAccountsService) RemoveExternalService(ctx context.Context, name string) error { diff --git a/pkg/services/serviceaccounts/extsvcaccounts/service_test.go b/pkg/services/serviceaccounts/extsvcaccounts/service_test.go index 33df87e283a..f2f9f8ba740 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/service_test.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/service_test.go @@ -4,9 +4,6 @@ import ( "context" "testing" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models/roletype" @@ -20,6 +17,8 @@ import ( sa "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) type TestEnv struct { @@ -441,3 +440,68 @@ func TestExtSvcAccountsService_RemoveExtSvcAccount(t *testing.T) { }) } } + +func TestExtSvcAccountsService_GetExternalServiceNames(t *testing.T) { + sa1 := sa.ServiceAccountDTO{ + Id: 1, + Name: sa.ExtSvcPrefix + "sa-svc-1", + Login: sa.ServiceAccountPrefix + sa.ExtSvcPrefix + "sa-svc-1", + OrgId: extsvcauth.TmpOrgID, + } + sa2 := sa.ServiceAccountDTO{ + Id: 2, + Name: sa.ExtSvcPrefix + "sa-svc-2", + Login: sa.ServiceAccountPrefix + sa.ExtSvcPrefix + "sa-svc-2", + OrgId: extsvcauth.TmpOrgID, + } + tests := []struct { + name string + init func(env *TestEnv) + want []string + }{ + { + name: "should return names", + init: func(env *TestEnv) { + env.SaSvc.On("SearchOrgServiceAccounts", mock.Anything, mock.MatchedBy(func(cmd *sa.SearchOrgServiceAccountsQuery) bool { + return cmd.OrgID == extsvcauth.TmpOrgID && + cmd.Filter == sa.FilterOnlyExternal && + len(cmd.SignedInUser.GetPermissions()[sa.ActionRead]) > 0 + })).Return(&sa.SearchOrgServiceAccountsResult{ + TotalCount: 2, + ServiceAccounts: []*sa.ServiceAccountDTO{&sa1, &sa2}, + Page: 1, + PerPage: 2, + }, nil) + }, + want: []string{"sa-svc-1", "sa-svc-2"}, + }, + { + name: "should handle nil search", + init: func(env *TestEnv) { + env.SaSvc.On("SearchOrgServiceAccounts", mock.Anything, mock.MatchedBy(func(cmd *sa.SearchOrgServiceAccountsQuery) bool { + return cmd.OrgID == extsvcauth.TmpOrgID && + cmd.Filter == sa.FilterOnlyExternal && + len(cmd.SignedInUser.GetPermissions()[sa.ActionRead]) > 0 + })).Return(nil, nil) + }, + want: []string{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + env := setupTestEnv(t) + if tt.init != nil { + tt.init(env) + } + + got, err := env.S.GetExternalServiceNames(ctx) + require.NoError(t, err) + + require.ElementsMatch(t, tt.want, got) + env.SaSvc.AssertExpectations(t) + env.AcStore.AssertExpectations(t) + }) + } +}