ExtSvcAuth: Refactor external service registry to use ExternalServiceRegistry variables (#78056)

ExtSvcAuth: Refactor external service registry to use ExternalServiceRegistry

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
 }

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 {
+func (f *FakeAuthService) HasExternalService(ctx context.Context, pluginID string) (bool, error) {
-	return f.Result != nil
+	return f.Result != nil, nil
 }
 
 func (f *FakeAuthService) RegisterExternalService(ctx context.Context, pluginID string, pType plugindef.Type, svc *plugindef.ExternalServiceRegistration) (*auth.ExternalService, error) {

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

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

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.

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
+	features featuremgmt.FeatureToggles
-	logger      log.Logger
+	logger   log.Logger
-	oauthServer oauthserver.OAuth2Server
+	oauthReg extsvcauth.ExternalServiceRegistry
-	saSvc       *extsvcaccounts.ExtSvcAccountsService
+	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,
+		oauthReg:        oauthServer,
-		saSvc:           saSvc,
+		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)
 	}

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)

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)