SSO: Add basic validation rules for LDAP settings (#89548)

add basic validation rules for LDAP settings
2024-06-21 15:48:19 +03:00
parent ab5a079bcc
commit 02b0b7af16
2 changed files with 133 additions and 0 deletions
@@ -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
 }

@@ -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)
+			}
+		})
+	}
+}