From 6987ad7b4d34defad09c616baa313206a0b113e9 Mon Sep 17 00:00:00 2001 From: Santiago Date: Tue, 2 Nov 2021 10:11:19 -0300 Subject: [PATCH] Fix issues with invalid Slack contact points (#41062) * Add validation when creating/updating a contact point * Change 201 status code for 200 (as it was before) --- pkg/api/alerting.go | 8 +++ pkg/services/alerting/service.go | 66 +++++++++++++++++ pkg/services/alerting/service_test.go | 82 +++++++++++++++++++--- pkg/services/alerting/test_notification.go | 42 ++--------- 4 files changed, 153 insertions(+), 45 deletions(-) diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index abd20d68c77..eb1e3ceea30 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -283,6 +283,10 @@ func CreateAlertNotification(c *models.ReqContext, cmd models.CreateAlertNotific if errors.Is(err, models.ErrAlertNotificationWithSameNameExists) || errors.Is(err, models.ErrAlertNotificationWithSameUIDExists) { return response.Error(409, "Failed to create alert notification", err) } + var alertingErr alerting.ValidationError + if errors.As(err, &alertingErr) { + return response.Error(400, err.Error(), err) + } return response.Error(500, "Failed to create alert notification", err) } @@ -301,6 +305,10 @@ func (hs *HTTPServer) UpdateAlertNotification(c *models.ReqContext, cmd models.U if errors.Is(err, models.ErrAlertNotificationNotFound) { return response.Error(404, err.Error(), err) } + var alertingErr alerting.ValidationError + if errors.As(err, &alertingErr) { + return response.Error(400, err.Error(), err) + } return response.Error(500, "Failed to update alert notification", err) } diff --git a/pkg/services/alerting/service.go b/pkg/services/alerting/service.go index 0a6b6e6192d..1623a6f9a74 100644 --- a/pkg/services/alerting/service.go +++ b/pkg/services/alerting/service.go @@ -52,6 +52,16 @@ func (s *AlertNotificationService) CreateAlertNotificationCommand(ctx context.Co return err } + model := models.AlertNotification{ + Name: cmd.Name, + Type: cmd.Type, + Settings: cmd.Settings, + } + + if err := s.validateAlertNotification(ctx, &model, cmd.SecureSettings); err != nil { + return err + } + return s.SQLStore.CreateAlertNotificationCommand(cmd) } @@ -62,6 +72,17 @@ func (s *AlertNotificationService) UpdateAlertNotification(ctx context.Context, return err } + model := models.AlertNotification{ + Id: cmd.Id, + Name: cmd.Name, + Type: cmd.Type, + Settings: cmd.Settings, + } + + if err := s.validateAlertNotification(ctx, &model, cmd.SecureSettings); err != nil { + return err + } + return s.SQLStore.UpdateAlertNotification(cmd) } @@ -100,3 +121,48 @@ func (s *AlertNotificationService) DeleteAlertNotificationWithUid(cmd *models.De func (s *AlertNotificationService) GetAlertNotificationsWithUidToSend(query *models.GetAlertNotificationsWithUidToSendQuery) error { return s.SQLStore.GetAlertNotificationsWithUidToSend(query) } + +func (s *AlertNotificationService) createNotifier(ctx context.Context, model *models.AlertNotification, secureSettings map[string]string) (Notifier, error) { + secureSettingsMap := map[string]string{} + + if model.Id > 0 { + query := &models.GetAlertNotificationsQuery{ + OrgId: model.OrgId, + Id: model.Id, + } + if err := s.SQLStore.GetAlertNotifications(query); err != nil { + return nil, err + } + + if query.Result != nil && query.Result.SecureSettings != nil { + var err error + secureSettingsMap, err = s.EncryptionService.DecryptJsonData(ctx, query.Result.SecureSettings, setting.SecretKey) + if err != nil { + return nil, err + } + } + } + + for k, v := range secureSettings { + secureSettingsMap[k] = v + } + + var err error + model.SecureSettings, err = s.EncryptionService.EncryptJsonData(ctx, secureSettingsMap, setting.SecretKey) + if err != nil { + return nil, err + } + + notifier, err := InitNotifier(model, s.EncryptionService.GetDecryptedValue) + if err != nil { + logger.Error("Failed to create notifier", "error", err.Error()) + return nil, err + } + + return notifier, nil +} + +func (s *AlertNotificationService) validateAlertNotification(ctx context.Context, model *models.AlertNotification, secureSettings map[string]string) error { + _, err := s.createNotifier(ctx, model, secureSettings) + return err +} diff --git a/pkg/services/alerting/service_test.go b/pkg/services/alerting/service_test.go index 7adf84ee453..0d6db35a07c 100644 --- a/pkg/services/alerting/service_test.go +++ b/pkg/services/alerting/service_test.go @@ -16,41 +16,107 @@ import ( func TestService(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) + nType := "test" + registerTestNotifier(nType) + s := ProvideService(bus.New(), sqlStore, ossencryption.ProvideService()) origSecret := setting.SecretKey setting.SecretKey = "alert_notification_service_test" + t.Cleanup(func() { setting.SecretKey = origSecret }) - var an *models.AlertNotification - - t.Run("create alert notification should encrypt the secure json data", func(t *testing.T) { + t.Run("create alert notification should reject an invalid command", func(t *testing.T) { ctx := context.Background() ss := map[string]string{"password": "12345"} cmd := models.CreateAlertNotificationCommand{SecureSettings: ss} + err := s.CreateAlertNotificationCommand(ctx, &cmd) + require.Error(t, err) + }) + + t.Run("create alert notification should encrypt the secure json data", func(t *testing.T) { + ctx := context.Background() + + ss := map[string]string{"password": "12345"} + cmd := models.CreateAlertNotificationCommand{SecureSettings: ss, Type: nType} + err := s.CreateAlertNotificationCommand(ctx, &cmd) require.NoError(t, err) - an = cmd.Result + an := cmd.Result decrypted, err := s.EncryptionService.DecryptJsonData(ctx, an.SecureSettings, setting.SecretKey) require.NoError(t, err) require.Equal(t, ss, decrypted) + + // Delete the created alert notification + delCmd := models.DeleteAlertNotificationCommand{ + Id: cmd.Result.Id, + OrgId: cmd.Result.OrgId, + } + err = s.DeleteAlertNotification(&delCmd) + require.NoError(t, err) + }) + + t.Run("update alert notification should reject an invalid command", func(t *testing.T) { + ctx := context.Background() + + // Save test notification. + ss := map[string]string{"password": "12345"} + createCmd := models.CreateAlertNotificationCommand{SecureSettings: ss, Type: nType} + + err := s.CreateAlertNotificationCommand(ctx, &createCmd) + require.NoError(t, err) + + // Try to update it with an invalid type. + updateCmd := models.UpdateAlertNotificationCommand{Id: createCmd.Result.Id, Settings: simplejson.New(), SecureSettings: ss, Type: "invalid"} + err = s.UpdateAlertNotification(ctx, &updateCmd) + require.Error(t, err) + + // Delete the created alert notification. + delCmd := models.DeleteAlertNotificationCommand{ + Id: createCmd.Result.Id, + OrgId: createCmd.Result.OrgId, + } + err = s.DeleteAlertNotification(&delCmd) + require.NoError(t, err) }) t.Run("update alert notification should encrypt the secure json data", func(t *testing.T) { ctx := context.Background() - ss := map[string]string{"password": "678910"} - cmd := models.UpdateAlertNotificationCommand{Id: an.Id, Settings: simplejson.New(), SecureSettings: ss} - err := s.UpdateAlertNotification(ctx, &cmd) + // Save test notification. + ss := map[string]string{"password": "12345"} + createCmd := models.CreateAlertNotificationCommand{SecureSettings: ss, Type: nType} + + err := s.CreateAlertNotificationCommand(ctx, &createCmd) require.NoError(t, err) - decrypted, err := s.EncryptionService.DecryptJsonData(ctx, cmd.Result.SecureSettings, setting.SecretKey) + // Update test notification. + updateCmd := models.UpdateAlertNotificationCommand{Id: createCmd.Result.Id, Settings: simplejson.New(), SecureSettings: ss, Type: nType} + err = s.UpdateAlertNotification(ctx, &updateCmd) + require.NoError(t, err) + + decrypted, err := s.EncryptionService.DecryptJsonData(ctx, updateCmd.Result.SecureSettings, setting.SecretKey) require.NoError(t, err) require.Equal(t, ss, decrypted) + + // Delete the created alert notification. + delCmd := models.DeleteAlertNotificationCommand{ + Id: createCmd.Result.Id, + OrgId: createCmd.Result.OrgId, + } + err = s.DeleteAlertNotification(&delCmd) + require.NoError(t, err) + }) +} + +func registerTestNotifier(notifierType string) { + RegisterNotifier(&NotifierPlugin{ + Type: notifierType, + Factory: func(*models.AlertNotification, GetDecryptedValueFn) (Notifier, error) { return nil, nil }, }) } diff --git a/pkg/services/alerting/test_notification.go b/pkg/services/alerting/test_notification.go index c6a3aa2f90f..01bc3cb087c 100644 --- a/pkg/services/alerting/test_notification.go +++ b/pkg/services/alerting/test_notification.go @@ -6,12 +6,10 @@ import ( "math/rand" "net/http" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/null" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/setting" ) // NotificationTestCommand initiates an test @@ -31,51 +29,21 @@ var ( ) func (s *AlertNotificationService) HandleNotificationTestCommand(ctx context.Context, cmd *NotificationTestCommand) error { - notifier := newNotificationService(nil, nil) + notificationSvc := newNotificationService(nil, nil) - model := &models.AlertNotification{ + model := models.AlertNotification{ + Id: cmd.ID, Name: cmd.Name, Type: cmd.Type, Settings: cmd.Settings, } - secureSettingsMap := map[string]string{} - - if cmd.ID > 0 { - query := &models.GetAlertNotificationsQuery{ - OrgId: cmd.OrgID, - Id: cmd.ID, - } - if err := bus.Dispatch(query); err != nil { - return err - } - - if query.Result.SecureSettings != nil { - var err error - secureSettingsMap, err = s.EncryptionService.DecryptJsonData(ctx, query.Result.SecureSettings, setting.SecretKey) - if err != nil { - return err - } - } - } - - for k, v := range cmd.SecureSettings { - secureSettingsMap[k] = v - } - - var err error - model.SecureSettings, err = s.EncryptionService.EncryptJsonData(ctx, secureSettingsMap, setting.SecretKey) + notifier, err := s.createNotifier(ctx, &model, cmd.SecureSettings) if err != nil { return err } - notifiers, err := InitNotifier(model, s.EncryptionService.GetDecryptedValue) - if err != nil { - logger.Error("Failed to create notifier", "error", err.Error()) - return err - } - - return notifier.sendNotifications(createTestEvalContext(cmd), notifierStateSlice{{notifier: notifiers}}) + return notificationSvc.sendNotifications(createTestEvalContext(cmd), notifierStateSlice{{notifier: notifier}}) } func createTestEvalContext(cmd *NotificationTestCommand) *EvalContext {