IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

SSO: run the validation on upsert with all secrets in settings (#86579)

Browse Source 
* run the validation on upsert with all secrets in settings

* rename social to reloadable
This commit is contained in:
Mihai Doarna 2024-04-23 16:50:16 +03:00 committed by GitHub
parent a6be12c037
commit bf15329492
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 116 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
pkg/services/ssosettings/ssosettingsimpl/service.go
View File

@ -192,24 +192,28 @@ func (s *Service) Upsert(ctx context.Context, settings *models.SSOSettings, requ
return ssosettings.ErrNotConfigurable
}
social, ok := s.reloadables[settings.Provider]
reloadable, ok := s.reloadables[settings.Provider]
if !ok {
return ssosettings.ErrInvalidProvider.Errorf("provider %s not found in reloadables", settings.Provider)
}
err := social.Validate(ctx, *settings, requester)
if err != nil {
return err
}
storedSettings, err := s.GetForProvider(ctx, settings.Provider)
if err != nil {
return err
}
secrets := collectSecrets(settings, storedSettings)
settingsWithSecrets, err := mergeSecrets(settings.Settings, storedSettings.Settings)
if err != nil {
return err
}
settings.Settings = settingsWithSecrets
settings.Settings, err = s.encryptSecrets(ctx, settings.Settings, storedSettings.Settings)
err = reloadable.Validate(ctx, *settings, requester)
if err != nil {
return err
}
settings.Settings, err = s.encryptSecrets(ctx, settings.Settings)
if err != nil {
return err
}
@ -221,9 +225,9 @@ func (s *Service) Upsert(ctx context.Context, settings *models.SSOSettings, requ
// make a copy of current settings for reload operation and apply overrides
reloadSettings := *settings
reloadSettings.Settings = overrideMaps(storedSettings.Settings, settings.Settings, secrets)
reloadSettings.Settings = overrideMaps(storedSettings.Settings, settingsWithSecrets)
go s.reload(social, settings.Provider, reloadSettings)
go s.reload(reloadable, settings.Provider, reloadSettings)
return nil
}
@ -317,7 +321,7 @@ func (s *Service) getFallbackStrategyFor(provider string) (ssosettings.FallbackS
return nil, false
}
func (s *Service) encryptSecrets(ctx context.Context, settings, storedSettings map[string]any) (map[string]any, error) {
func (s *Service) encryptSecrets(ctx context.Context, settings map[string]any) (map[string]any, error) {
result := make(map[string]any)
for k, v := range settings {
if isSecret(k) && v != "" {
@ -326,10 +330,6 @@ func (s *Service) encryptSecrets(ctx context.Context, settings, storedSettings m
return result, fmt.Errorf("failed to encrypt %s setting because it is not a string: %v", k, v)
}
if !isNewSecretValue(strValue) {
strValue = storedSettings[k].(string)
}
encryptedSecret, err := s.secrets.Encrypt(ctx, []byte(strValue), secrets.WithoutScope())
if err != nil {
return result, err
@ -483,20 +483,26 @@ func mergeSettings(storedSettings, systemSettings map[string]any) map[string]any
return settings
}
// collectSecrets collects all the secrets from the request and the currently stored settings
// and returns a new map
func collectSecrets(settings *models.SSOSettings, storedSettings *models.SSOSettings) map[string]any {
secrets := map[string]any{}
for k, v := range settings.Settings {
// mergeSecrets returns a new map with the current value for secrets that have not been updated
func mergeSecrets(settings map[string]any, storedSettings map[string]any) (map[string]any, error) {
settingsWithSecrets := map[string]any{}
for k, v := range settings {
if isSecret(k) {
if isNewSecretValue(v.(string)) {
secrets[k] = v.(string) // use the new value
strValue, ok := v.(string)
if !ok {
return nil, fmt.Errorf("secret value is not a string")
}
if isNewSecretValue(strValue) {
settingsWithSecrets[k] = strValue // use the new value
continue
}
secrets[k] = storedSettings.Settings[k] // keep the currently stored value
settingsWithSecrets[k] = storedSettings[k] // keep the currently stored value
} else {
settingsWithSecrets[k] = v
}
}
return secrets
return settingsWithSecrets, nil
}
func overrideMaps(maps ...map[string]any) map[string]any {

89
pkg/services/ssosettings/ssosettingsimpl/service_test.go
View File

@ -979,6 +979,29 @@ func TestService_Upsert(t *testing.T) {
require.Error(t, err)
})
t.Run("returns error if a secret does not have the type string", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t, false, false, nil)
provider := social.OktaProviderName
settings := models.SSOSettings{
Provider: provider,
Settings: map[string]any{
"client_id": "client-id",
"client_secret": 123,
"enabled": true,
},
IsDeleted: false,
}
reloadable := ssosettingstests.NewMockReloadable(t)
env.reloadables[provider] = reloadable
err := env.service.Upsert(context.Background(), &settings, &user.SignedInUser{})
require.Error(t, err)
})
t.Run("returns error if secrets encryption failed", func(t *testing.T) {
t.Parallel()
@ -1027,8 +1050,15 @@ func TestService_Upsert(t *testing.T) {
},
}
expected := settings
expected.Settings = make(map[string]any)
for key, value := range settings.Settings {
expected.Settings[key] = value
}
expected.Settings["client_secret"] = "encrypted-client-secret"
reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(nil)
reloadable.On("Validate", mock.Anything, expected, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
env.reloadables[provider] = reloadable
env.secrets.On("Decrypt", mock.Anything, []byte("current-client-secret"), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
@ -1037,8 +1067,61 @@ func TestService_Upsert(t *testing.T) {
err := env.service.Upsert(context.Background(), &settings, &user.SignedInUser{})
require.NoError(t, err)
settings.Settings["client_secret"] = base64.RawStdEncoding.EncodeToString([]byte("current-client-secret"))
require.EqualValues(t, settings, env.store.ActualSSOSettings)
expected.Settings["client_secret"] = base64.RawStdEncoding.EncodeToString([]byte("current-client-secret"))
require.EqualValues(t, expected, env.store.ActualSSOSettings)
})
t.Run("run validation with all new and current secrets available in settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
Provider: provider,
Settings: map[string]any{
"client_secret": setting.RedactedPassword,
"private_key": setting.RedactedPassword,
"certificate": "new-certificate",
},
IsDeleted: false,
}
env.store.ExpectedSSOSetting = &models.SSOSettings{
Provider: provider,
Settings: map[string]any{
"client_secret": base64.RawStdEncoding.EncodeToString([]byte("encrypted-current-client-secret")),
"private_key": base64.RawStdEncoding.EncodeToString([]byte("encrypted-current-private-key")),
"certificate": base64.RawStdEncoding.EncodeToString([]byte("encrypted-current-certificate")),
},
}
expected := settings
expected.Settings = make(map[string]any)
for key, value := range settings.Settings {
expected.Settings[key] = value
}
expected.Settings["client_secret"] = "current-client-secret"
expected.Settings["private_key"] = "current-private-key"
reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, expected, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
env.reloadables[provider] = reloadable
env.secrets.On("Decrypt", mock.Anything, []byte("encrypted-current-client-secret"), mock.Anything).Return([]byte("current-client-secret"), nil).Once()
env.secrets.On("Decrypt", mock.Anything, []byte("encrypted-current-private-key"), mock.Anything).Return([]byte("current-private-key"), nil).Once()
env.secrets.On("Decrypt", mock.Anything, []byte("encrypted-current-certificate"), mock.Anything).Return([]byte("current-certificate"), nil).Once()
env.secrets.On("Encrypt", mock.Anything, []byte("current-client-secret"), mock.Anything).Return([]byte("encrypted-current-client-secret"), nil).Once()
env.secrets.On("Encrypt", mock.Anything, []byte("current-private-key"), mock.Anything).Return([]byte("encrypted-current-private-key"), nil).Once()
env.secrets.On("Encrypt", mock.Anything, []byte("new-certificate"), mock.Anything).Return([]byte("encrypted-new-certificate"), nil).Once()
err := env.service.Upsert(context.Background(), &settings, &user.SignedInUser{})
require.NoError(t, err)
expected.Settings["client_secret"] = base64.RawStdEncoding.EncodeToString([]byte("encrypted-current-client-secret"))
expected.Settings["private_key"] = base64.RawStdEncoding.EncodeToString([]byte("encrypted-current-private-key"))
expected.Settings["certificate"] = base64.RawStdEncoding.EncodeToString([]byte("encrypted-new-certificate"))
require.EqualValues(t, expected, env.store.ActualSSOSettings)
})
t.Run("returns error if store failed to upsert settings", func(t *testing.T) {