Auth: Remove types from SSOSettings struct in SSO service (#79459)

* replace SSOSettings with SSOSettingsDTO

* fix database tests

* fix oauth strategy

* fix sso settings service tests

* add secrets encryption on update

* rename SSOSettingsDTO to SSOSettings

* remove extraKeys from strategy

* change back settings type from createOAuthConnector to OAuthInfo

* do not parse multi-value fields in oauth strategy

pkg/services/ssosettings/ssosettingsimpl/service.go

@@ -3,11 +3,12 @@ package ssosettingsimpl
 import (
 	"context"
 	"errors"
+	"fmt"
+	"strings"
 
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/login/social"
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/secrets"
@@ -95,12 +96,12 @@ func (s *SSOSettingsService) List(ctx context.Context) ([]*models.SSOSettings, e
 		settings := getSettingsByProvider(provider, storedSettings)
 		if len(settings) == 0 {
 			// If there is no data in the DB then we need to load the settings using the fallback strategy
-			setting, err := s.loadSettingsUsingFallbackStrategy(ctx, provider)
+			fallbackSettings, err := s.loadSettingsUsingFallbackStrategy(ctx, provider)
 			if err != nil {
 				return nil, err
 			}
 
-			settings = append(settings, setting)
+			settings = append(settings, fallbackSettings)
 		}
 		result = append(result, settings...)
 	}
@@ -109,23 +110,16 @@ func (s *SSOSettingsService) List(ctx context.Context) ([]*models.SSOSettings, e
 }
 
 func (s *SSOSettingsService) Upsert(ctx context.Context, settings models.SSOSettings) error {
+	var err error
 	// TODO: also check whether the provider is configurable
 	// Get the connector for the provider (from the reloadables) and call Validate
 
-	if isOAuthProvider(settings.Provider) {
-		encryptedClientSecret, err := s.secrets.Encrypt(ctx, []byte(settings.OAuthSettings.ClientSecret), secrets.WithoutScope())
-		if err != nil {
-			return err
-		}
-		settings.OAuthSettings.ClientSecret = string(encryptedClientSecret)
-	}
-
-	err := s.store.Upsert(ctx, settings)
+	settings.Settings, err = s.encryptSecrets(ctx, settings.Settings)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	return s.store.Upsert(ctx, settings)
 }
 
 func (s *SSOSettingsService) Patch(ctx context.Context, provider string, data map[string]any) error {
@@ -162,16 +156,11 @@ func (s *SSOSettingsService) loadSettingsUsingFallbackStrategy(ctx context.Conte
 		return nil, err
 	}
 
-	switch settingsFromSystem := settingsFromSystem.(type) {
-	case *social.OAuthInfo:
-		return &models.SSOSettings{
-			Provider:      provider,
-			Source:        models.System,
-			OAuthSettings: settingsFromSystem,
-		}, nil
-	default:
-		return nil, errors.New("could not parse settings from system")
-	}
+	return &models.SSOSettings{
+		Provider: provider,
+		Source:   models.System,
+		Settings: settingsFromSystem,
+	}, nil
 }
 
 func getSettingsByProvider(provider string, settings []*models.SSOSettings) []*models.SSOSettings {
@@ -193,12 +182,32 @@ func (s *SSOSettingsService) getFallBackstrategyFor(provider string) (ssosetting
 	return nil, false
 }
 
-func isOAuthProvider(provider string) bool {
-	for _, oAuthProvider := range ssosettings.AllOAuthProviders {
-		if oAuthProvider == provider {
-			return true
+func (s *SSOSettingsService) encryptSecrets(ctx context.Context, settings map[string]any) (map[string]any, error) {
+	secretFieldPatterns := []string{"secret"}
+
+	isSecret := func(field string) bool {
+		for _, v := range secretFieldPatterns {
+			if strings.Contains(strings.ToLower(field), strings.ToLower(v)) {
+				return true
+			}
+		}
+		return false
+	}
+
+	for k, v := range settings {
+		if isSecret(k) {
+			strValue, ok := v.(string)
+			if !ok {
+				return settings, fmt.Errorf("failed to encrypt %s setting because it is not a string: %v", k, v)
+			}
+
+			encryptedSecret, err := s.secrets.Encrypt(ctx, []byte(strValue), secrets.WithoutScope())
+			if err != nil {
+				return settings, err
+			}
+			settings[k] = string(encryptedSecret)
 		}
 	}
 
-	return false
+	return settings, nil
 }

pkg/services/ssosettings/ssosettingsimpl/service_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/login/social"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	secretsFakes "github.com/grafana/grafana/pkg/services/secrets/fakes"
@@ -31,14 +30,14 @@ func TestSSOSettingsService_GetForProvider(t *testing.T) {
 			name: "should return successfully",
 			setup: func(env testEnv) {
 				env.store.ExpectedSSOSetting = &models.SSOSettings{
-					Provider:      "github",
-					OAuthSettings: &social.OAuthInfo{Enabled: true},
-					Source:        models.DB,
+					Provider: "github",
+					Settings: map[string]any{"enabled": true},
+					Source:   models.DB,
 				}
 			},
 			want: &models.SSOSettings{
-				Provider:      "github",
-				OAuthSettings: &social.OAuthInfo{Enabled: true},
+				Provider: "github",
+				Settings: map[string]any{"enabled": true},
 			},
 			wantErr: false,
 		},
@@ -53,12 +52,12 @@ func TestSSOSettingsService_GetForProvider(t *testing.T) {
 			setup: func(env testEnv) {
 				env.store.ExpectedError = ssosettings.ErrNotFound
 				env.fallbackStrategy.ExpectedIsMatch = true
-				env.fallbackStrategy.ExpectedConfig = &social.OAuthInfo{Enabled: true}
+				env.fallbackStrategy.ExpectedConfig = map[string]any{"enabled": true}
 			},
 			want: &models.SSOSettings{
-				Provider:      "github",
-				OAuthSettings: &social.OAuthInfo{Enabled: true},
-				Source:        models.System,
+				Provider: "github",
+				Settings: map[string]any{"enabled": true},
+				Source:   models.System,
 			},
 			wantErr: false,
 		},
@@ -115,54 +114,54 @@ func TestSSOSettingsService_List(t *testing.T) {
 			setup: func(env testEnv) {
 				env.store.ExpectedSSOSettings = []*models.SSOSettings{
 					{
-						Provider:      "github",
-						OAuthSettings: &social.OAuthInfo{Enabled: true},
-						Source:        models.DB,
+						Provider: "github",
+						Settings: map[string]any{"enabled": true},
+						Source:   models.DB,
 					},
 					{
-						Provider:      "okta",
-						OAuthSettings: &social.OAuthInfo{Enabled: false},
-						Source:        models.DB,
+						Provider: "okta",
+						Settings: map[string]any{"enabled": false},
+						Source:   models.DB,
 					},
 				}
 				env.fallbackStrategy.ExpectedIsMatch = true
-				env.fallbackStrategy.ExpectedConfig = &social.OAuthInfo{Enabled: false}
+				env.fallbackStrategy.ExpectedConfig = map[string]any{"enabled": false}
 			},
 			want: []*models.SSOSettings{
 				{
-					Provider:      "github",
-					OAuthSettings: &social.OAuthInfo{Enabled: true},
-					Source:        models.DB,
+					Provider: "github",
+					Settings: map[string]any{"enabled": true},
+					Source:   models.DB,
 				},
 				{
-					Provider:      "okta",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.DB,
+					Provider: "okta",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.DB,
 				},
 				{
-					Provider:      "gitlab",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "gitlab",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "generic_oauth",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "generic_oauth",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "google",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "google",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "azuread",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "azuread",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "grafana_com",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "grafana_com",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 			},
 			wantErr: false,
@@ -178,43 +177,43 @@ func TestSSOSettingsService_List(t *testing.T) {
 			setup: func(env testEnv) {
 				env.store.ExpectedSSOSettings = []*models.SSOSettings{}
 				env.fallbackStrategy.ExpectedIsMatch = true
-				env.fallbackStrategy.ExpectedConfig = &social.OAuthInfo{Enabled: false}
+				env.fallbackStrategy.ExpectedConfig = map[string]any{"enabled": false}
 			},
 			want: []*models.SSOSettings{
 				{
-					Provider:      "github",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "github",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "okta",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "okta",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "gitlab",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "gitlab",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "generic_oauth",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "generic_oauth",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "google",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "google",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "azuread",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "azuread",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 				{
-					Provider:      "grafana_com",
-					OAuthSettings: &social.OAuthInfo{Enabled: false},
-					Source:        models.System,
+					Provider: "grafana_com",
+					Settings: map[string]any{"enabled": false},
+					Source:   models.System,
 				},
 			},
 			wantErr: false,
@@ -255,15 +254,15 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
 
 		settings := models.SSOSettings{
 			Provider: "azuread",
-			OAuthSettings: &social.OAuthInfo{
-				ClientId:     "client-id",
-				ClientSecret: "client-secret",
-				Enabled:      true,
+			Settings: map[string]any{
+				"client_id":     "client-id",
+				"client_secret": "client-secret",
+				"enabled":       true,
 			},
 			IsDeleted: false,
 		}
 
-		env.secrets.On("Encrypt", mock.Anything, []byte(settings.OAuthSettings.ClientSecret), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
+		env.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
 
 		err := env.service.Upsert(context.Background(), settings)
 		require.NoError(t, err)
@@ -274,15 +273,15 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
 
 		settings := models.SSOSettings{
 			Provider: "azuread",
-			OAuthSettings: &social.OAuthInfo{
-				ClientId:     "client-id",
-				ClientSecret: "client-secret",
-				Enabled:      true,
+			Settings: map[string]any{
+				"client_id":     "client-id",
+				"client_secret": "client-secret",
+				"enabled":       true,
 			},
 			IsDeleted: false,
 		}
 
-		env.secrets.On("Encrypt", mock.Anything, []byte(settings.OAuthSettings.ClientSecret), mock.Anything).Return(nil, errors.New("encryption failed")).Once()
+		env.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return(nil, errors.New("encryption failed")).Once()
 
 		err := env.service.Upsert(context.Background(), settings)
 		require.Error(t, err)
@@ -293,15 +292,15 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
 
 		settings := models.SSOSettings{
 			Provider: "azuread",
-			OAuthSettings: &social.OAuthInfo{
-				ClientId:     "client-id",
-				ClientSecret: "client-secret",
-				Enabled:      true,
+			Settings: map[string]any{
+				"client_id":     "client-id",
+				"client_secret": "client-secret",
+				"enabled":       true,
 			},
 			IsDeleted: false,
 		}
 
-		env.secrets.On("Encrypt", mock.Anything, []byte(settings.OAuthSettings.ClientSecret), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
+		env.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
 		env.store.ExpectedError = errors.New("upsert failed")
 
 		err := env.service.Upsert(context.Background(), settings)