From 2cf73012ab69b01b7d4529d5c203c4e343c6e032 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:27:04 +0100 Subject: [PATCH] [v11.0.x] SSO: fix reloading settings when a provider contains empty settings (#86142) SSO: fix reloading settings when a provider contains empty settings (#85102) * fix reloading settings when a provider contains empty settings * do not increment reloadFailures if settings are empty (cherry picked from commit fad6dc4db11cdb15fc31e847938d95c9192232fe) Co-authored-by: Mihai Doarna --- .../ssosettings/ssosettingsimpl/service.go | 9 ++++-- .../ssosettingsimpl/service_test.go | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/services/ssosettings/ssosettingsimpl/service.go b/pkg/services/ssosettings/ssosettingsimpl/service.go index 1d4ae45f55e..a592ca9b99c 100644 --- a/pkg/services/ssosettings/ssosettingsimpl/service.go +++ b/pkg/services/ssosettings/ssosettingsimpl/service.go @@ -365,9 +365,14 @@ func (s *Service) doReload(ctx context.Context) { } for provider, connector := range s.reloadables { - setting := getSettingByProvider(provider, settingsList) + settings := getSettingByProvider(provider, settingsList) - err = connector.Reload(ctx, *setting) + if settings == nil || len(settings.Settings) == 0 { + s.logger.Warn("SSO Settings is empty", "provider", provider) + continue + } + + err = connector.Reload(ctx, *settings) if err != nil { s.metrics.reloadFailures.WithLabelValues(provider).Inc() s.logger.Error("failed to reload SSO Settings", "provider", provider, "err", err) diff --git a/pkg/services/ssosettings/ssosettingsimpl/service_test.go b/pkg/services/ssosettings/ssosettingsimpl/service_test.go index 41066118016..6bd7206c719 100644 --- a/pkg/services/ssosettings/ssosettingsimpl/service_test.go +++ b/pkg/services/ssosettings/ssosettingsimpl/service_test.go @@ -1248,6 +1248,36 @@ func TestService_DoReload(t *testing.T) { env.service.doReload(context.Background()) }) + t.Run("successfully reload settings when some providers have empty settings", func(t *testing.T) { + t.Parallel() + + env := setupTestEnv(t, false, false, nil) + + settingsList := []*models.SSOSettings{ + { + Provider: "azuread", + Settings: map[string]any{ + "enabled": true, + "client_id": "azuread_client_id", + }, + }, + { + Provider: "google", + Settings: map[string]any{}, + }, + } + env.store.ExpectedSSOSettings = settingsList + + reloadable := ssosettingstests.NewMockReloadable(t) + reloadable.On("Reload", mock.Anything, *settingsList[0]).Return(nil).Once() + env.reloadables["azuread"] = reloadable + + // registers a provider with empty settings + env.reloadables["github"] = nil + + env.service.doReload(context.Background()) + }) + t.Run("failed fetching the SSO settings", func(t *testing.T) { t.Parallel()