SSO: Add oldSettings param to the Validate function from SSO settings (#88245)

* add oldSettings param to the Validate function from SSO settings

* update unit tests adding the missing param to Validate
This commit is contained in:
Mihai Doarna 2024-05-31 11:08:52 +03:00 committed by GitHub
parent 9d8052830f
commit e1aedb65b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 29 additions and 29 deletions

View File

@ -191,7 +191,7 @@ func (s *SocialAzureAD) Reload(ctx context.Context, settings ssoModels.SSOSettin
return nil return nil
} }
func (s *SocialAzureAD) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialAzureAD) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -1130,7 +1130,7 @@ func TestSocialAzureAD_Validate(t *testing.T) {
if tc.requester == nil { if tc.requester == nil {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -75,7 +75,7 @@ func NewGenericOAuthProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMa
return provider return provider
} }
func (s *SocialGenericOAuth) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialGenericOAuth) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -1090,7 +1090,7 @@ func TestSocialGenericOAuth_Validate(t *testing.T) {
if tc.requester == nil { if tc.requester == nil {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -82,7 +82,7 @@ func NewGitHubProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMapper *
return provider return provider
} }
func (s *SocialGithub) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialGithub) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -501,7 +501,7 @@ func TestSocialGitHub_Validate(t *testing.T) {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -66,7 +66,7 @@ func NewGitLabProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMapper *
return provider return provider
} }
func (s *SocialGitlab) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialGitlab) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -582,7 +582,7 @@ func TestSocialGitlab_Validate(t *testing.T) {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -65,7 +65,7 @@ func NewGoogleProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMapper *
return provider return provider
} }
func (s *SocialGoogle) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialGoogle) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -803,7 +803,7 @@ func TestSocialGoogle_Validate(t *testing.T) {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -56,7 +56,7 @@ func NewGrafanaComProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMapp
return provider return provider
} }
func (s *SocialGrafanaCom) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialGrafanaCom) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -205,7 +205,7 @@ func TestSocialGrafanaCom_Validate(t *testing.T) {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.expectError { if tc.expectError {
require.Error(t, err) require.Error(t, err)
} else { } else {

View File

@ -62,7 +62,7 @@ func NewOktaProvider(info *social.OAuthInfo, cfg *setting.Cfg, orgRoleMapper *Or
return provider return provider
} }
func (s *SocialOkta) Validate(ctx context.Context, settings ssoModels.SSOSettings, requester identity.Requester) error { func (s *SocialOkta) Validate(ctx context.Context, settings ssoModels.SSOSettings, _ ssoModels.SSOSettings, requester identity.Requester) error {
info, err := CreateOAuthInfoFromKeyValues(settings.Settings) info, err := CreateOAuthInfoFromKeyValues(settings.Settings)
if err != nil { if err != nil {
return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) return ssosettings.ErrInvalidSettings.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err)

View File

@ -287,7 +287,7 @@ func TestSocialOkta_Validate(t *testing.T) {
if tc.requester == nil { if tc.requester == nil {
tc.requester = &user.SignedInUser{IsGrafanaAdmin: false} tc.requester = &user.SignedInUser{IsGrafanaAdmin: false}
} }
err := s.Validate(context.Background(), tc.settings, tc.requester) err := s.Validate(context.Background(), tc.settings, ssoModels.SSOSettings{}, tc.requester)
if tc.wantErr != nil { if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr) require.ErrorIs(t, err, tc.wantErr)
return return

View File

@ -41,7 +41,7 @@ type Service interface {
//go:generate mockery --name Reloadable --structname MockReloadable --outpkg ssosettingstests --filename reloadable_mock.go --output ./ssosettingstests/ //go:generate mockery --name Reloadable --structname MockReloadable --outpkg ssosettingstests --filename reloadable_mock.go --output ./ssosettingstests/
type Reloadable interface { type Reloadable interface {
Reload(ctx context.Context, settings models.SSOSettings) error Reload(ctx context.Context, settings models.SSOSettings) error
Validate(ctx context.Context, settings models.SSOSettings, requester identity.Requester) error Validate(ctx context.Context, settings models.SSOSettings, oldSettings models.SSOSettings, requester identity.Requester) error
} }
// FallbackStrategy is an interface that can be implemented to allow a provider to load settings from a different source // FallbackStrategy is an interface that can be implemented to allow a provider to load settings from a different source

View File

@ -200,7 +200,7 @@ func (s *Service) Upsert(ctx context.Context, settings *models.SSOSettings, requ
} }
settings.Settings = settingsWithSecrets settings.Settings = settingsWithSecrets
err = reloadable.Validate(ctx, *settings, requester) err = reloadable.Validate(ctx, *settings, *storedSettings, requester)
if err != nil { if err != nil {
return err return err
} }

View File

@ -892,7 +892,7 @@ func TestService_Upsert(t *testing.T) {
wg.Add(1) wg.Add(1)
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, settings, mock.Anything, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.MatchedBy(func(settings models.SSOSettings) bool { reloadable.On("Reload", mock.Anything, mock.MatchedBy(func(settings models.SSOSettings) bool {
defer wg.Done() defer wg.Done()
return settings.Provider == provider && return settings.Provider == provider &&
@ -1000,7 +1000,7 @@ func TestService_Upsert(t *testing.T) {
} }
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(errors.New("validation failed")) reloadable.On("Validate", mock.Anything, settings, mock.Anything, mock.Anything).Return(errors.New("validation failed"))
env.reloadables[provider] = reloadable env.reloadables[provider] = reloadable
err := env.service.Upsert(context.Background(), &settings, &user.SignedInUser{}) err := env.service.Upsert(context.Background(), &settings, &user.SignedInUser{})
@ -1068,7 +1068,7 @@ func TestService_Upsert(t *testing.T) {
} }
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, settings, mock.Anything, mock.Anything).Return(nil)
env.reloadables[provider] = reloadable 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() env.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return(nil, errors.New("encryption failed")).Once()
@ -1107,7 +1107,7 @@ func TestService_Upsert(t *testing.T) {
expected.Settings["client_secret"] = "encrypted-client-secret" expected.Settings["client_secret"] = "encrypted-client-secret"
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, expected, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, expected, mock.Anything, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe() reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
env.reloadables[provider] = reloadable env.reloadables[provider] = reloadable
env.secrets.On("Decrypt", mock.Anything, []byte("current-client-secret"), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once() env.secrets.On("Decrypt", mock.Anything, []byte("current-client-secret"), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
@ -1154,7 +1154,7 @@ func TestService_Upsert(t *testing.T) {
expected.Settings["private_key"] = "current-private-key" expected.Settings["private_key"] = "current-private-key"
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, expected, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, expected, mock.Anything, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe() reloadable.On("Reload", mock.Anything, mock.Anything).Return(nil).Maybe()
env.reloadables[provider] = reloadable 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-client-secret"), mock.Anything).Return([]byte("current-client-secret"), nil).Once()
@ -1190,7 +1190,7 @@ func TestService_Upsert(t *testing.T) {
} }
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, settings, mock.Anything, mock.Anything).Return(nil)
env.reloadables[provider] = reloadable 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.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()
env.store.GetFn = func(ctx context.Context, provider string) (*models.SSOSettings, error) { env.store.GetFn = func(ctx context.Context, provider string) (*models.SSOSettings, error) {
@ -1222,7 +1222,7 @@ func TestService_Upsert(t *testing.T) {
} }
reloadable := ssosettingstests.NewMockReloadable(t) reloadable := ssosettingstests.NewMockReloadable(t)
reloadable.On("Validate", mock.Anything, settings, mock.Anything).Return(nil) reloadable.On("Validate", mock.Anything, settings, mock.Anything, mock.Anything).Return(nil)
reloadable.On("Reload", mock.Anything, mock.Anything).Return(errors.New("failed reloading new settings")).Maybe() reloadable.On("Reload", mock.Anything, mock.Anything).Return(errors.New("failed reloading new settings")).Maybe()
env.reloadables[provider] = reloadable 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.secrets.On("Encrypt", mock.Anything, []byte(settings.Settings["client_secret"].(string)), mock.Anything).Return([]byte("encrypted-client-secret"), nil).Once()

View File

@ -1,4 +1,4 @@
// Code generated by mockery v2.40.1. DO NOT EDIT. // Code generated by mockery v2.42.1. DO NOT EDIT.
package ssosettingstests package ssosettingstests
@ -34,17 +34,17 @@ func (_m *MockReloadable) Reload(ctx context.Context, settings models.SSOSetting
return r0 return r0
} }
// Validate provides a mock function with given fields: ctx, settings, requester // Validate provides a mock function with given fields: ctx, settings, oldSettings, requester
func (_m *MockReloadable) Validate(ctx context.Context, settings models.SSOSettings, requester identity.Requester) error { func (_m *MockReloadable) Validate(ctx context.Context, settings models.SSOSettings, oldSettings models.SSOSettings, requester identity.Requester) error {
ret := _m.Called(ctx, settings, requester) ret := _m.Called(ctx, settings, oldSettings, requester)
if len(ret) == 0 { if len(ret) == 0 {
panic("no return value specified for Validate") panic("no return value specified for Validate")
} }
var r0 error var r0 error
if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings, identity.Requester) error); ok { if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings, models.SSOSettings, identity.Requester) error); ok {
r0 = rf(ctx, settings, requester) r0 = rf(ctx, settings, oldSettings, requester)
} else { } else {
r0 = ret.Error(0) r0 = ret.Error(0)
} }