From 53e9bf47db21bace6783f2d0eb777209bcc725c6 Mon Sep 17 00:00:00 2001 From: Guilherme Caulada Date: Mon, 25 Apr 2022 15:12:44 -0300 Subject: [PATCH] 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 | 2 + .../service/datasource_service_test.go | 100 ++++++++++++++---- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index f1d1e7c1870..7eb21d0562c 100644 --- a/pkg/api/datasources.go +++ b/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 diff --git a/pkg/services/datasources/service/datasource_service_test.go b/pkg/services/datasources/service/datasource_service_test.go index 747a34be797..e250315fa73 100644 --- a/pkg/services/datasources/service/datasource_service_test.go +++ b/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() - json.Set("tlsAuthWithCACert", true) + sjson := simplejson.New() + 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() - json.Set("tlsAuth", true) + sjson := simplejson.New() + 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() - json.Set("tlsAuthWithCACert", true) - json.Set("serverName", "server-name") + sjson := simplejson.New() + sjson.Set("tlsAuthWithCACert", true) + 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() - json.Set("tlsSkipVerify", true) + sjson := simplejson.New() + 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