diff --git a/pkg/services/ldap/service/ldap.go b/pkg/services/ldap/service/ldap.go index 2125b322cb6..d3d87e9b2fc 100644 --- a/pkg/services/ldap/service/ldap.go +++ b/pkg/services/ldap/service/ldap.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "fmt" "sync" "github.com/grafana/grafana/pkg/apimachinery/identity" @@ -108,6 +109,27 @@ func (s *LDAPImpl) Reload(ctx context.Context, settings models.SSOSettings) erro } func (s *LDAPImpl) Validate(ctx context.Context, settings models.SSOSettings, oldSettings models.SSOSettings, requester identity.Requester) error { + ldapCfg, err := resolveServerConfig(settings.Settings["config"]) + if err != nil { + return err + } + + enabled := resolveBool(settings.Settings["enabled"], false) + if !enabled { + return nil + } + + if len(ldapCfg.Servers) == 0 { + return fmt.Errorf("no servers configured for LDAP") + } + + for i, server := range ldapCfg.Servers { + // host is required for every LDAP server config + if server.Host == "" { + return fmt.Errorf("no host configured for server with index %d", i) + } + } + return nil } diff --git a/pkg/services/ldap/service/ldap_test.go b/pkg/services/ldap/service/ldap_test.go index 74cb4ab1a8d..6cf33fc1bff 100644 --- a/pkg/services/ldap/service/ldap_test.go +++ b/pkg/services/ldap/service/ldap_test.go @@ -297,3 +297,114 @@ func TestReload(t *testing.T) { }) } } + +func TestValidate(t *testing.T) { + testCases := []struct { + description string + settings models.SSOSettings + isValid bool + containsError string + }{ + { + description: "successfully validate basic settings", + settings: models.SSOSettings{ + Provider: "ldap", + Settings: map[string]any{ + "enabled": true, + "config": map[string]any{ + "servers": []any{ + map[string]any{ + "host": "127.0.0.1", + }, + }, + }, + }, + }, + isValid: true, + }, + { + description: "successfully validate settings that are not enabled", + settings: models.SSOSettings{ + Provider: "ldap", + Settings: map[string]any{ + "enabled": false, + "config": map[string]any{ + "servers": []any{ + map[string]any{ + "port": 123, + }, + }, + }, + }, + }, + isValid: true, + }, + { + description: "validation fails for invalid settings", + settings: models.SSOSettings{ + Provider: "ldap", + Settings: map[string]any{ + "enabled": true, + "config": map[string]any{ + "servers": "invalid server config", + }, + }, + }, + isValid: false, + containsError: "cannot unmarshal", + }, + { + description: "validation fails when no servers are configured", + settings: models.SSOSettings{ + Provider: "ldap", + Settings: map[string]any{ + "enabled": true, + "config": map[string]any{ + "servers": []any{}, + }, + }, + }, + isValid: false, + containsError: "no servers configured", + }, + { + description: "validation fails if one server does not have a host configured", + settings: models.SSOSettings{ + Provider: "ldap", + Settings: map[string]any{ + "enabled": true, + "config": map[string]any{ + "servers": []any{ + map[string]any{ + "host": "127.0.0.1", + }, + map[string]any{ + "port": 123, + }, + }, + }, + }, + }, + isValid: false, + containsError: "no host configured", + }, + } + + for _, tt := range testCases { + t.Run(tt.description, func(t *testing.T) { + ldapImpl := &LDAPImpl{ + features: featuremgmt.WithManager(featuremgmt.FlagSsoSettingsApi), + loadingMutex: &sync.Mutex{}, + } + + err := ldapImpl.Validate(context.Background(), tt.settings, models.SSOSettings{}, nil) + + if tt.isValid { + require.NoError(t, err) + } else { + require.Error(t, err) + require.ErrorContains(t, err, tt.containsError) + } + }) + } +}