Auth: Implement the SSO Settings update endpoint (#79676)
* merge with system settings before storing them in the db * add base for validating sso settings * add unit tests for sso settings validation * call Reload() from sso service upsert() * remove actual validation because it was moved in a separate pr * use constant to fix lint error * check if provider is configurable in service Upsert() method * add unit tests for update provider settings api method * fix lint error
This commit is contained in:
@@ -110,16 +110,47 @@ 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 !isProviderConfigurable(settings.Provider) {
|
||||
return ssosettings.ErrInvalidProvider.Errorf("provider %s is not configurable", settings.Provider)
|
||||
}
|
||||
|
||||
social, ok := s.reloadables[settings.Provider]
|
||||
if !ok {
|
||||
return ssosettings.ErrInvalidProvider.Errorf("provider %s not found in reloadables", settings.Provider)
|
||||
}
|
||||
|
||||
err := social.Validate(ctx, settings)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
systemSettings, err := s.loadSettingsUsingFallbackStrategy(ctx, settings.Provider)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// add the SSO settings from system that are not available in the user input
|
||||
// in order to have a complete set of SSO settings for every provider in the database
|
||||
settings.Settings = mergeSettings(settings.Settings, systemSettings.Settings)
|
||||
|
||||
settings.Settings, err = s.encryptSecrets(ctx, settings.Settings)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return s.store.Upsert(ctx, settings)
|
||||
err = s.store.Upsert(ctx, settings)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
go func() {
|
||||
err = social.Reload(context.Background(), settings)
|
||||
if err != nil {
|
||||
s.log.Error("failed to reload the provider", "provider", settings.Provider, "error", err)
|
||||
}
|
||||
}()
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *SSOSettingsService) Patch(ctx context.Context, provider string, data map[string]any) error {
|
||||
@@ -183,31 +214,60 @@ func (s *SSOSettingsService) getFallBackstrategyFor(provider string) (ssosetting
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
result := make(map[string]any)
|
||||
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)
|
||||
return result, 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
|
||||
return result, err
|
||||
}
|
||||
settings[k] = string(encryptedSecret)
|
||||
result[k] = string(encryptedSecret)
|
||||
} else {
|
||||
result[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
return settings, nil
|
||||
return result, nil
|
||||
}
|
||||
|
||||
func isSecret(fieldName string) bool {
|
||||
secretFieldPatterns := []string{"secret"}
|
||||
|
||||
for _, v := range secretFieldPatterns {
|
||||
if strings.Contains(strings.ToLower(fieldName), strings.ToLower(v)) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func mergeSettings(apiSettings, systemSettings map[string]any) map[string]any {
|
||||
settings := make(map[string]any)
|
||||
|
||||
for k, v := range apiSettings {
|
||||
settings[k] = v
|
||||
}
|
||||
|
||||
for k, v := range systemSettings {
|
||||
if _, ok := settings[k]; !ok {
|
||||
settings[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
return settings
|
||||
}
|
||||
|
||||
func isProviderConfigurable(provider string) bool {
|
||||
for _, configurable := range ssosettings.ConfigurableOAuthProviders {
|
||||
if provider == configurable {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ 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"
|
||||
@@ -252,8 +253,9 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
|
||||
t.Run("successfully upsert SSO settings", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.AzureADProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: "azuread",
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
@@ -262,17 +264,180 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
env.reloadables[provider] = reloadable
|
||||
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)
|
||||
|
||||
settings.Settings["client_secret"] = "encrypted-client-secret"
|
||||
require.EqualValues(t, settings, env.store.ActualSSOSettings)
|
||||
})
|
||||
|
||||
t.Run("successfully upsert SSO settings having system settings", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.GitHubProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
systemSettings := map[string]any{
|
||||
"api_url": "http://api-url",
|
||||
"use_refresh_token": true,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
env.reloadables[provider] = reloadable
|
||||
env.fallbackStrategy.ExpectedConfig = systemSettings
|
||||
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)
|
||||
|
||||
settings.Settings["client_secret"] = "encrypted-client-secret"
|
||||
settings.Settings["api_url"] = systemSettings["api_url"]
|
||||
settings.Settings["use_refresh_token"] = systemSettings["use_refresh_token"]
|
||||
require.EqualValues(t, settings, env.store.ActualSSOSettings)
|
||||
})
|
||||
|
||||
t.Run("successfully upsert SSO settings having system settings without overwriting user settings", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.GitlabProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
systemSettings := map[string]any{
|
||||
"client_id": "client-id-from-system",
|
||||
"client_secret": "client-secret-from-system",
|
||||
"enabled": false,
|
||||
"api_url": "http://api-url",
|
||||
"use_refresh_token": true,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
|
||||
env.reloadables[provider] = reloadable
|
||||
env.fallbackStrategy.ExpectedConfig = systemSettings
|
||||
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)
|
||||
|
||||
settings.Settings["client_secret"] = "encrypted-client-secret"
|
||||
settings.Settings["api_url"] = systemSettings["api_url"]
|
||||
settings.Settings["use_refresh_token"] = systemSettings["use_refresh_token"]
|
||||
require.EqualValues(t, settings, env.store.ActualSSOSettings)
|
||||
})
|
||||
|
||||
t.Run("returns error if provider is not configurable", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.GrafanaComProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
env.reloadables[provider] = reloadable
|
||||
|
||||
err := env.service.Upsert(context.Background(), settings)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("returns error if provider was not found in reloadables", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.AzureADProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
// the reloadable is available for other provider
|
||||
env.reloadables["github"] = reloadable
|
||||
|
||||
err := env.service.Upsert(context.Background(), settings)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("returns error if validation fails", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.AzureADProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(errors.New("validation failed"))
|
||||
env.reloadables[provider] = reloadable
|
||||
|
||||
err := env.service.Upsert(context.Background(), settings)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("returns error if a fallback strategy is not available for the provider", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
settings := models.SSOSettings{
|
||||
Provider: social.AzureADProviderName,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
env.fallbackStrategy.ExpectedIsMatch = false
|
||||
|
||||
err := env.service.Upsert(context.Background(), settings)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("returns error if secrets encryption failed", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.OktaProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: "azuread",
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
@@ -281,6 +446,9 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
env.reloadables[provider] = reloadable
|
||||
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)
|
||||
@@ -290,8 +458,9 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
|
||||
t.Run("returns error if store failed to upsert settings", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.AzureADProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: "azuread",
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
@@ -300,19 +469,49 @@ func TestSSOSettingsService_Upsert(t *testing.T) {
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
env.reloadables[provider] = reloadable
|
||||
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)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("successfully upsert SSO settings if reload fails", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := social.AzureADProviderName
|
||||
settings := models.SSOSettings{
|
||||
Provider: provider,
|
||||
Settings: map[string]any{
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
"enabled": true,
|
||||
},
|
||||
IsDeleted: false,
|
||||
}
|
||||
|
||||
reloadable := ssosettingstests.NewMockReloadable(t)
|
||||
reloadable.On("Validate", mock.Anything, settings).Return(nil)
|
||||
reloadable.On("Reload", mock.Anything, mock.Anything).Return(errors.New("failed reloading new settings")).Maybe()
|
||||
env.reloadables[provider] = reloadable
|
||||
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)
|
||||
|
||||
settings.Settings["client_secret"] = "encrypted-client-secret"
|
||||
require.EqualValues(t, settings, env.store.ActualSSOSettings)
|
||||
})
|
||||
}
|
||||
|
||||
func TestSSOSettingsService_Delete(t *testing.T) {
|
||||
t.Run("successfully delete SSO settings", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := "azuread"
|
||||
provider := social.AzureADProviderName
|
||||
env.store.ExpectedError = nil
|
||||
|
||||
err := env.service.Delete(context.Background(), provider)
|
||||
@@ -322,7 +521,7 @@ func TestSSOSettingsService_Delete(t *testing.T) {
|
||||
t.Run("SSO settings not found for the specified provider", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := "azuread"
|
||||
provider := social.AzureADProviderName
|
||||
env.store.ExpectedError = ssosettings.ErrNotFound
|
||||
|
||||
err := env.service.Delete(context.Background(), provider)
|
||||
@@ -333,7 +532,7 @@ func TestSSOSettingsService_Delete(t *testing.T) {
|
||||
t.Run("store fails to delete the SSO settings for the specified provider", func(t *testing.T) {
|
||||
env := setupTestEnv(t)
|
||||
|
||||
provider := "azuread"
|
||||
provider := social.AzureADProviderName
|
||||
env.store.ExpectedError = errors.New("delete sso settings failed")
|
||||
|
||||
err := env.service.Delete(context.Background(), provider)
|
||||
@@ -347,13 +546,16 @@ func setupTestEnv(t *testing.T) testEnv {
|
||||
fallbackStrategy := ssosettingstests.NewFakeFallbackStrategy()
|
||||
secrets := secretsFakes.NewMockService(t)
|
||||
accessControl := acimpl.ProvideAccessControl(setting.NewCfg())
|
||||
reloadables := make(map[string]ssosettings.Reloadable)
|
||||
|
||||
fallbackStrategy.ExpectedIsMatch = true
|
||||
|
||||
svc := &SSOSettingsService{
|
||||
log: log.NewNopLogger(),
|
||||
store: store,
|
||||
ac: accessControl,
|
||||
fbStrategies: []ssosettings.FallbackStrategy{fallbackStrategy},
|
||||
reloadables: make(map[string]ssosettings.Reloadable),
|
||||
reloadables: reloadables,
|
||||
secrets: secrets,
|
||||
}
|
||||
|
||||
@@ -363,6 +565,7 @@ func setupTestEnv(t *testing.T) testEnv {
|
||||
ac: accessControl,
|
||||
fallbackStrategy: fallbackStrategy,
|
||||
secrets: secrets,
|
||||
reloadables: reloadables,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -372,4 +575,5 @@ type testEnv struct {
|
||||
ac accesscontrol.AccessControl
|
||||
fallbackStrategy *ssosettingstests.FakeFallbackStrategy
|
||||
secrets *secretsFakes.MockService
|
||||
reloadables map[string]ssosettings.Reloadable
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user