diff --git a/pkg/plugins/auth/models.go b/pkg/plugins/auth/models.go index cc5dc32b81a..f336da46b3e 100644 --- a/pkg/plugins/auth/models.go +++ b/pkg/plugins/auth/models.go @@ -13,7 +13,7 @@ type ExternalService struct { } type ExternalServiceRegistry interface { - HasExternalService(ctx context.Context, pluginID string) bool + HasExternalService(ctx context.Context, pluginID string) (bool, error) RegisterExternalService(ctx context.Context, pluginID string, pType plugindef.Type, svc *plugindef.ExternalServiceRegistration) (*ExternalService, error) RemoveExternalService(ctx context.Context, pluginID string) error } diff --git a/pkg/plugins/manager/fakes/fakes.go b/pkg/plugins/manager/fakes/fakes.go index a5ea3761285..d33a61d27dc 100644 --- a/pkg/plugins/manager/fakes/fakes.go +++ b/pkg/plugins/manager/fakes/fakes.go @@ -437,8 +437,8 @@ type FakeAuthService struct { Result *auth.ExternalService } -func (f *FakeAuthService) HasExternalService(ctx context.Context, pluginID string) bool { - return f.Result != nil +func (f *FakeAuthService) HasExternalService(ctx context.Context, pluginID string) (bool, error) { + return f.Result != nil, nil } func (f *FakeAuthService) RegisterExternalService(ctx context.Context, pluginID string, pType plugindef.Type, svc *plugindef.ExternalServiceRegistration) (*auth.ExternalService, error) { diff --git a/pkg/plugins/manager/installer.go b/pkg/plugins/manager/installer.go index a12454718f5..e4f26dc54c8 100644 --- a/pkg/plugins/manager/installer.go +++ b/pkg/plugins/manager/installer.go @@ -160,10 +160,11 @@ func (m *PluginInstaller) Remove(ctx context.Context, pluginID string) error { } } - if m.serviceRegistry.HasExternalService(ctx, pluginID) { + has, err := m.serviceRegistry.HasExternalService(ctx, pluginID) + if err == nil && has { return m.serviceRegistry.RemoveExternalService(ctx, pluginID) } - return nil + return err } // plugin finds a plugin with `pluginID` from the store diff --git a/pkg/services/extsvcauth/models.go b/pkg/services/extsvcauth/models.go index 6b1f3e278a7..c1f5765e7a6 100644 --- a/pkg/services/extsvcauth/models.go +++ b/pkg/services/extsvcauth/models.go @@ -18,7 +18,7 @@ type AuthProvider string type ExternalServiceRegistry interface { // HasExternalService returns whether an external service has been saved with that name. - HasExternalService(ctx context.Context, name string) bool + HasExternalService(ctx context.Context, name string) (bool, 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/oasimpl/service.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go index 2c807dbf257..cfe694676dd 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go @@ -119,6 +119,16 @@ func newProvider(config *fosite.Config, storage any, signingKeyService signingke ) } +// HasExternalService returns whether an external service has been saved with that name. +func (s *OAuth2ServiceImpl) HasExternalService(ctx context.Context, name string) (bool, error) { + client, errRetrieve := s.sqlstore.GetExternalServiceByName(ctx, name) + if errRetrieve != nil && !errors.Is(errRetrieve, oauthserver.ErrClientNotFound) { + return false, errRetrieve + } + + return client != nil, nil +} + // GetExternalService retrieves an external service from store by client_id. It populates the SelfPermissions and // SignedInUser from the associated service account. // For performance reason, the service uses caching. diff --git a/pkg/services/extsvcauth/registry/service.go b/pkg/services/extsvcauth/registry/service.go index 44d29827689..6a718494336 100644 --- a/pkg/services/extsvcauth/registry/service.go +++ b/pkg/services/extsvcauth/registry/service.go @@ -7,7 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/services/extsvcauth" - "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" + "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oasimpl" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/serviceaccounts/extsvcaccounts" ) @@ -15,30 +15,30 @@ import ( var _ extsvcauth.ExternalServiceRegistry = &Registry{} type Registry struct { - features featuremgmt.FeatureToggles - logger log.Logger - oauthServer oauthserver.OAuth2Server - saSvc *extsvcaccounts.ExtSvcAccountsService + features featuremgmt.FeatureToggles + logger log.Logger + oauthReg extsvcauth.ExternalServiceRegistry + saReg extsvcauth.ExternalServiceRegistry extSvcProviders map[string]extsvcauth.AuthProvider lock sync.Mutex } -func ProvideExtSvcRegistry(oauthServer oauthserver.OAuth2Server, saSvc *extsvcaccounts.ExtSvcAccountsService, features featuremgmt.FeatureToggles) *Registry { +func ProvideExtSvcRegistry(oauthServer *oasimpl.OAuth2ServiceImpl, saSvc *extsvcaccounts.ExtSvcAccountsService, features featuremgmt.FeatureToggles) *Registry { return &Registry{ extSvcProviders: map[string]extsvcauth.AuthProvider{}, features: features, lock: sync.Mutex{}, logger: log.New("extsvcauth.registry"), - oauthServer: oauthServer, - saSvc: saSvc, + oauthReg: oauthServer, + saReg: saSvc, } } // HasExternalService returns whether an external service has been saved with that name. -func (r *Registry) HasExternalService(ctx context.Context, name string) bool { +func (r *Registry) HasExternalService(ctx context.Context, name string) (bool, error) { _, ok := r.extSvcProviders[slugify.Slugify(name)] - return ok + return ok, nil } // RemoveExternalService removes an external service and its associated resources from the database (ex: service account, token). @@ -56,14 +56,14 @@ func (r *Registry) RemoveExternalService(ctx context.Context, name string) error return nil } r.logger.Debug("Routing External Service removal to the External Service Account service", "service", name) - return r.saSvc.RemoveExternalService(ctx, name) + return r.saReg.RemoveExternalService(ctx, name) case extsvcauth.OAuth2Server: if !r.features.IsEnabled(featuremgmt.FlagExternalServiceAuth) { r.logger.Debug("Skipping External Service removal, flag disabled", "service", name, "flag", featuremgmt.FlagExternalServiceAccounts) return nil } r.logger.Debug("Routing External Service removal to the OAuth2Server", "service", name) - return r.oauthServer.RemoveExternalService(ctx, name) + return r.oauthReg.RemoveExternalService(ctx, name) default: return extsvcauth.ErrUnknownProvider.Errorf("unknow provider '%v'", provider) } @@ -85,14 +85,14 @@ func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.Exte return nil, nil } r.logger.Debug("Routing the External Service registration to the External Service Account service", "service", cmd.Name) - return r.saSvc.SaveExternalService(ctx, cmd) + return r.saReg.SaveExternalService(ctx, cmd) case extsvcauth.OAuth2Server: if !r.features.IsEnabled(featuremgmt.FlagExternalServiceAuth) { r.logger.Warn("Skipping External Service authentication, flag disabled", "service", cmd.Name, "flag", featuremgmt.FlagExternalServiceAuth) return nil, nil } r.logger.Debug("Routing the External Service registration to the OAuth2Server", "service", cmd.Name) - return r.oauthServer.SaveExternalService(ctx, cmd) + return r.oauthReg.SaveExternalService(ctx, cmd) default: return nil, extsvcauth.ErrUnknownProvider.Errorf("unknow provider '%v'", cmd.AuthProvider) } diff --git a/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go b/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go index 6b030185120..aee54ca352c 100644 --- a/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go +++ b/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go @@ -31,10 +31,10 @@ func ProvideService(cfg *config.Cfg, reg extsvcauth.ExternalServiceRegistry, set return s } -func (s *Service) HasExternalService(ctx context.Context, pluginID string) bool { +func (s *Service) HasExternalService(ctx context.Context, pluginID string) (bool, error) { if !s.featureEnabled { s.log.Debug("Skipping HasExternalService call. The feature is behind a feature toggle and needs to be enabled.") - return false + return false, nil } return s.reg.HasExternalService(ctx, pluginID) diff --git a/pkg/services/serviceaccounts/extsvcaccounts/service.go b/pkg/services/serviceaccounts/extsvcaccounts/service.go index 4cd92c8dfb3..85c5a26c951 100644 --- a/pkg/services/serviceaccounts/extsvcaccounts/service.go +++ b/pkg/services/serviceaccounts/extsvcaccounts/service.go @@ -64,6 +64,18 @@ func (esa *ExtSvcAccountsService) EnableExtSvcAccount(ctx context.Context, cmd * return esa.saSvc.EnableServiceAccount(ctx, cmd.OrgID, saID, cmd.Enabled) } +// HasExternalService returns whether an external service has been saved with that name. +func (esa *ExtSvcAccountsService) HasExternalService(ctx context.Context, name string) (bool, error) { + saName := sa.ExtSvcPrefix + slugify.Slugify(name) + + saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, extsvcauth.TmpOrgID, saName) + if errRetrieve != nil && !errors.Is(errRetrieve, sa.ErrServiceAccountNotFound) { + return false, errRetrieve + } + + return saID > 0, nil +} + // RetrieveExtSvcAccount fetches an external service account by ID func (esa *ExtSvcAccountsService) RetrieveExtSvcAccount(ctx context.Context, orgID, saID int64) (*sa.ExtSvcAccount, error) { svcAcc, err := esa.saSvc.RetrieveServiceAccount(ctx, orgID, saID)