Secrets: Implement tests and debug log improvements on unified secrets (#48213)

* Add test for decrypted values on datasource service

* Add debug log when fail to parse secure json fields

* Fix minor import issue

* Refactor encJson to json and simplejson to sjson on tests

pkg/api/datasources.go

@@ -487,6 +487,8 @@ func (hs *HTTPServer) convertModelToDtos(ctx context.Context, ds *models.DataSou
 				dto.SecureJsonFields[k] = true
 			}
 		}
+	} else {
+		datasourcesLogger.Debug("Failed to retrieve datasource secrets to parse secure json fields", "error", err)
 	}
 
 	return dto

pkg/services/datasources/service/datasource_service_test.go

@@ -2,7 +2,7 @@ package service
 
 import (
 	"context"
-	encJson "encoding/json"
+	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -11,6 +11,7 @@ import (
 
 	"github.com/grafana/grafana-azure-sdk-go/azsettings"
 	sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
+	"github.com/grafana/grafana/pkg/services/secrets"
 	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
@@ -224,8 +225,8 @@ func TestService_GetHttpTransport(t *testing.T) {
 
 		setting.SecretKey = "password"
 
-		json := simplejson.New()
+		sjson := simplejson.New()
-		json.Set("tlsAuthWithCACert", true)
+		sjson.Set("tlsAuthWithCACert", true)
 
 		secretsStore := kvstore.SetupTestService(t)
 		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -272,8 +273,8 @@ func TestService_GetHttpTransport(t *testing.T) {
 
 		setting.SecretKey = "password"
 
-		json := simplejson.New()
+		sjson := simplejson.New()
-		json.Set("tlsAuth", true)
+		sjson.Set("tlsAuth", true)
 
 		secretsStore := kvstore.SetupTestService(t)
 		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -285,10 +286,10 @@ func TestService_GetHttpTransport(t *testing.T) {
 			Name:     "kubernetes",
 			Url:      "http://k8s:8001",
 			Type:     "Kubernetes",
-			JsonData: json,
+			JsonData: sjson,
 		}
 
-		secureJsonData, err := encJson.Marshal(map[string]string{
+		secureJsonData, err := json.Marshal(map[string]string{
 			"tlsClientCert": clientCert,
 			"tlsClientKey":  clientKey,
 		})
@@ -316,9 +317,9 @@ func TestService_GetHttpTransport(t *testing.T) {
 
 		setting.SecretKey = "password"
 
-		json := simplejson.New()
+		sjson := simplejson.New()
-		json.Set("tlsAuthWithCACert", true)
+		sjson.Set("tlsAuthWithCACert", true)
-		json.Set("serverName", "server-name")
+		sjson.Set("serverName", "server-name")
 
 		secretsStore := kvstore.SetupTestService(t)
 		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -330,10 +331,10 @@ func TestService_GetHttpTransport(t *testing.T) {
 			Name:     "kubernetes",
 			Url:      "http://k8s:8001",
 			Type:     "Kubernetes",
-			JsonData: json,
+			JsonData: sjson,
 		}
 
-		secureJsonData, err := encJson.Marshal(map[string]string{
+		secureJsonData, err := json.Marshal(map[string]string{
 			"tlsCACert": caCert,
 		})
 		require.NoError(t, err)
@@ -359,8 +360,8 @@ func TestService_GetHttpTransport(t *testing.T) {
 			},
 		})
 
-		json := simplejson.New()
+		sjson := simplejson.New()
-		json.Set("tlsSkipVerify", true)
+		sjson.Set("tlsSkipVerify", true)
 
 		secretsStore := kvstore.SetupTestService(t)
 		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -370,7 +371,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 			Id:       1,
 			Url:      "http://k8s:8001",
 			Type:     "Kubernetes",
-			JsonData: json,
+			JsonData: sjson,
 		}
 
 		rt1, err := dsService.GetHTTPTransport(context.Background(), &ds, provider)
@@ -390,7 +391,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 	t.Run("Should set custom headers if configured in JsonData", func(t *testing.T) {
 		provider := httpclient.NewProvider()
 
-		json := simplejson.NewFromAny(map[string]interface{}{
+		sjson := simplejson.NewFromAny(map[string]interface{}{
 			"httpHeaderName1": "Authorization",
 		})
 
@@ -404,10 +405,10 @@ func TestService_GetHttpTransport(t *testing.T) {
 			Name:     "kubernetes",
 			Url:      "http://k8s:8001",
 			Type:     "Kubernetes",
-			JsonData: json,
+			JsonData: sjson,
 		}
 
-		secureJsonData, err := encJson.Marshal(map[string]string{
+		secureJsonData, err := json.Marshal(map[string]string{
 			"httpHeaderValue1": "Bearer xf5yhfkpsnmgo",
 		})
 		require.NoError(t, err)
@@ -415,7 +416,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 		err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(secureJsonData))
 		require.NoError(t, err)
 
-		headers := dsService.getCustomHeaders(json, map[string]string{"httpHeaderValue1": "Bearer xf5yhfkpsnmgo"})
+		headers := dsService.getCustomHeaders(sjson, map[string]string{"httpHeaderValue1": "Bearer xf5yhfkpsnmgo"})
 		require.Equal(t, "Bearer xf5yhfkpsnmgo", headers["Authorization"])
 
 		// 1. Start HTTP test server which checks the request headers
@@ -456,7 +457,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 	t.Run("Should use request timeout if configured in JsonData", func(t *testing.T) {
 		provider := httpclient.NewProvider()
 
-		json := simplejson.NewFromAny(map[string]interface{}{
+		sjson := simplejson.NewFromAny(map[string]interface{}{
 			"timeout": 19,
 		})
 
@@ -468,7 +469,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 			Id:       1,
 			Url:      "http://k8s:8001",
 			Type:     "Kubernetes",
-			JsonData: json,
+			JsonData: sjson,
 		}
 
 		client, err := dsService.GetHTTPClient(context.Background(), &ds, provider)
@@ -491,7 +492,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 			setting.SigV4AuthEnabled = origSigV4Enabled
 		})
 
-		json, err := simplejson.NewJson([]byte(`{ "sigV4Auth": true }`))
+		sjson, err := simplejson.NewJson([]byte(`{ "sigV4Auth": true }`))
 		require.NoError(t, err)
 
 		secretsStore := kvstore.SetupTestService(t)
@@ -500,7 +501,7 @@ func TestService_GetHttpTransport(t *testing.T) {
 
 		ds := models.DataSource{
 			Type:     models.DS_ES,
-			JsonData: json,
+			JsonData: sjson,
 		}
 
 		_, err = dsService.GetHTTPTransport(context.Background(), &ds, provider)
@@ -706,6 +707,59 @@ func TestService_HTTPClientOptions(t *testing.T) {
 	})
 }
 
+func TestService_GetDecryptedValues(t *testing.T) {
+	t.Run("should migrate and retrieve values from secure json data", func(t *testing.T) {
+		ds := &models.DataSource{
+			Id:   1,
+			Url:  "https://api.example.com",
+			Type: "prometheus",
+		}
+
+		secretsStore := kvstore.SetupTestService(t)
+		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
+		dsService := ProvideService(nil, secretsService, secretsStore, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewPermissionsServicesMock())
+
+		jsonData := map[string]string{
+			"password": "securePassword",
+		}
+		secureJsonData, err := dsService.SecretsService.EncryptJsonData(context.Background(), jsonData, secrets.WithoutScope())
+
+		require.NoError(t, err)
+		ds.SecureJsonData = secureJsonData
+
+		values, err := dsService.DecryptedValues(context.Background(), ds)
+		require.NoError(t, err)
+
+		require.Equal(t, jsonData, values)
+	})
+
+	t.Run("should retrieve values from secret store", func(t *testing.T) {
+		ds := &models.DataSource{
+			Id:   1,
+			Url:  "https://api.example.com",
+			Type: "prometheus",
+		}
+
+		secretsStore := kvstore.SetupTestService(t)
+		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
+		dsService := ProvideService(nil, secretsService, secretsStore, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewPermissionsServicesMock())
+
+		jsonData := map[string]string{
+			"password": "securePassword",
+		}
+		jsonString, err := json.Marshal(jsonData)
+		require.NoError(t, err)
+
+		err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(jsonString))
+		require.NoError(t, err)
+
+		values, err := dsService.DecryptedValues(context.Background(), ds)
+		require.NoError(t, err)
+
+		require.Equal(t, jsonData, values)
+	})
+}
+
 const caCert string = `-----BEGIN CERTIFICATE-----
 MIIDATCCAemgAwIBAgIJAMQ5hC3CPDTeMA0GCSqGSIb3DQEBCwUAMBcxFTATBgNV
 BAMMDGNhLWs4cy1zdGhsbTAeFw0xNjEwMjcwODQyMjdaFw00NDAzMTQwODQyMjda