Alerting: idempotent decrypt\encrypt operations on extra configs (#107592)

* add prefix to encrypted string to distinguish between unencrypted and encrypted ones

---------

Co-authored-by: Alexander Akhmetov <me@alx.cx>

pkg/services/ngalert/notifier/crypto.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
@@ -14,6 +15,16 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets"
 )
 
+const (
+	// encryptedContentPrefix is a marker that identifies encrypted Alertmanager configurations.
+	// When this prefix is present at the beginning of a configuration string:
+	// 1. During encryption: It indicates the content is already encrypted and should be skipped
+	// 2. During decryption: It indicates the content (minus this prefix) should be base64 decoded
+	//    and then decrypted using the secrets service
+	// This prefix helps maintain idempotency in encryption/decryption operations.
+	cryptoPrefix = "crypto_"
+)
+
 // Crypto allows decryption of Alertmanager Configuration and encryption of arbitrary payloads.
 type Crypto interface {
 	LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver) error
@@ -253,12 +264,17 @@ func NewExtraConfigsCrypto(secrets secretService) *ExtraConfigsCrypto {
 
 func (c *ExtraConfigsCrypto) EncryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error {
 	for i := range config.ExtraConfigs {
+		// If it has prefix, consider it encrypted already
+		if strings.HasPrefix(config.ExtraConfigs[i].AlertmanagerConfig, cryptoPrefix) {
+			continue
+		}
+
 		encryptedValue, err := c.secrets.Encrypt(ctx, []byte(config.ExtraConfigs[i].AlertmanagerConfig), secrets.WithoutScope())
 		if err != nil {
 			return fmt.Errorf("failed to encrypt extra configuration: %w", err)
 		}
 
-		config.ExtraConfigs[i].AlertmanagerConfig = base64.StdEncoding.EncodeToString(encryptedValue)
+		config.ExtraConfigs[i].AlertmanagerConfig = cryptoPrefix + base64.StdEncoding.EncodeToString(encryptedValue)
 	}
 
 	return nil
@@ -266,12 +282,15 @@ func (c *ExtraConfigsCrypto) EncryptExtraConfigs(ctx context.Context, config *de
 
 func (c *ExtraConfigsCrypto) DecryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error {
 	for i := range config.ExtraConfigs {
-		// Check if the config is encrypted by trying to base64 decode it
-		encryptedValue, err := base64.StdEncoding.DecodeString(config.ExtraConfigs[i].AlertmanagerConfig)
-		if err != nil {
-			// If it can't be base64 decoded, assume it's already decrypted and skip
+		// If it does not have prefix, consider it decrypted already
+		if !strings.HasPrefix(config.ExtraConfigs[i].AlertmanagerConfig, cryptoPrefix) {
 			continue
 		}
+		// Check if the config is encrypted by trying to base64 decode it
+		encryptedValue, err := base64.StdEncoding.DecodeString(config.ExtraConfigs[i].AlertmanagerConfig[len(cryptoPrefix):])
+		if err != nil {
+			return fmt.Errorf("failed to decode extra configuration: %w", err)
+		}
 
 		decryptedValue, err := c.secrets.Decrypt(ctx, encryptedValue)
 		if err != nil {

pkg/services/ngalert/notifier/crypto_test.go

@@ -0,0 +1,106 @@
+package notifier
+
+import (
+	"context"
+	"encoding/base64"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
+	"github.com/grafana/grafana/pkg/services/secrets/fakes"
+)
+
+func TestEncryptExtraConfigs(t *testing.T) {
+	config := "plain-text-config"
+	encryptedConfig := base64.StdEncoding.EncodeToString([]byte(config))
+	tests := []struct {
+		name           string
+		inputConfig    string
+		expectedConfig string
+	}{
+		{
+			name:           "Encrypts unencrypted configs",
+			inputConfig:    config,
+			expectedConfig: cryptoPrefix + encryptedConfig,
+		},
+		{
+			name:           "Skips already encrypted configs",
+			inputConfig:    cryptoPrefix + "very-encrypted-data",
+			expectedConfig: cryptoPrefix + "very-encrypted-data",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := fakes.NewFakeSecretsService()
+
+			c := &alertmanagerCrypto{
+				secrets: m,
+			}
+
+			cfg := &definitions.PostableUserConfig{
+				ExtraConfigs: []definitions.ExtraConfiguration{
+					{AlertmanagerConfig: tt.inputConfig},
+				},
+			}
+
+			err := c.EncryptExtraConfigs(context.Background(), cfg)
+
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedConfig, cfg.ExtraConfigs[0].AlertmanagerConfig)
+		})
+	}
+}
+
+func TestDecryptExtraConfigs(t *testing.T) {
+	decryptedData := "derypted-data"
+	decryptedDataBase64 := base64.StdEncoding.EncodeToString([]byte(decryptedData))
+	tests := []struct {
+		name           string
+		inputConfig    string
+		expectedError  string
+		expectedConfig string
+	}{
+		{
+			name:           "Decrypts encrypted configs",
+			inputConfig:    cryptoPrefix + decryptedDataBase64,
+			expectedConfig: decryptedData,
+		},
+		{
+			name:           "Skips already encrypted configs",
+			inputConfig:    "very-decrypted-data",
+			expectedConfig: "very-decrypted-data",
+		},
+		{
+			name:          "Fails if not base64 encoded",
+			inputConfig:   cryptoPrefix + "plain-text-config",
+			expectedError: "failed to decode extra configuration",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := fakes.NewFakeSecretsService()
+			c := &alertmanagerCrypto{
+				secrets: m,
+			}
+
+			cfg := &definitions.PostableUserConfig{
+				ExtraConfigs: []definitions.ExtraConfiguration{
+					{AlertmanagerConfig: tt.inputConfig},
+				},
+			}
+
+			err := c.DecryptExtraConfigs(context.Background(), cfg)
+
+			if tt.expectedError != "" {
+				require.ErrorContains(t, err, tt.expectedError)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedConfig, cfg.ExtraConfigs[0].AlertmanagerConfig)
+		})
+	}
+}