From a912c970e376322ec874cf7edae5d82afda4be29 Mon Sep 17 00:00:00 2001 From: Hugo Kiyodi Oshiro Date: Thu, 27 Jul 2023 11:11:43 +0200 Subject: [PATCH] Provisioning: Fix overwrite SecureJSONData on provisioning (#72395) * Overwrite SecureJSONData on provisioning --- pkg/services/datasources/models.go | 1 + .../datasources/service/datasource.go | 8 +- .../datasources/service/datasource_test.go | 90 +++++++++++++++++++ .../provisioning/datasources/types.go | 33 +++---- 4 files changed, 113 insertions(+), 19 deletions(-) diff --git a/pkg/services/datasources/models.go b/pkg/services/datasources/models.go index b472b0f597d..101a789b13a 100644 --- a/pkg/services/datasources/models.go +++ b/pkg/services/datasources/models.go @@ -134,6 +134,7 @@ type UpdateDataSourceCommand struct { ReadOnly bool `json:"-"` EncryptedSecureJsonData map[string][]byte `json:"-"` UpdateSecretFn UpdateSecretFn `json:"-"` + IgnoreOldSecureJsonData bool `json:"-"` } // DeleteDataSourceCommand will delete a DataSource based on OrgID as well as the UID (preferred), ID, or Name. diff --git a/pkg/services/datasources/service/datasource.go b/pkg/services/datasources/service/datasource.go index cdfad67cd63..22eee0656cb 100644 --- a/pkg/services/datasources/service/datasource.go +++ b/pkg/services/datasources/service/datasource.go @@ -644,9 +644,11 @@ func (s *Service) fillWithSecureJSONData(ctx context.Context, cmd *datasources.U cmd.SecureJsonData = make(map[string]string) } - for k, v := range decrypted { - if _, ok := cmd.SecureJsonData[k]; !ok { - cmd.SecureJsonData[k] = v + if !cmd.IgnoreOldSecureJsonData { + for k, v := range decrypted { + if _, ok := cmd.SecureJsonData[k]; !ok { + cmd.SecureJsonData[k] = v + } } } diff --git a/pkg/services/datasources/service/datasource_test.go b/pkg/services/datasources/service/datasource_test.go index e830d2e04f3..a9049ad1275 100644 --- a/pkg/services/datasources/service/datasource_test.go +++ b/pkg/services/datasources/service/datasource_test.go @@ -187,6 +187,96 @@ func TestService_UpdateDataSource(t *testing.T) { _, err = dsService.UpdateDataSource(context.Background(), cmd) require.ErrorIs(t, err, datasources.ErrDataSourceNameExists) }) + + t.Run("should merge cmd.SecureJsonData with db data", func(t *testing.T) { + sqlStore := db.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + quotaService := quotatest.New(false, nil) + mockPermission := acmock.NewMockedPermissionsService() + dsService, err := ProvideService(sqlStore, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), actest.FakeAccessControl{}, mockPermission, quotaService) + require.NoError(t, err) + + mockPermission.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil) + + expectedDbKey := "db-secure-key" + expectedDbValue := "db-secure-value" + ds, err := dsService.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + OrgID: 1, + Name: "test-datasource", + SecureJsonData: map[string]string{ + expectedDbKey: expectedDbValue, + }, + }) + require.NoError(t, err) + + expectedOgKey := "cmd-secure-key" + expectedOgValue := "cmd-secure-value" + + cmd := &datasources.UpdateDataSourceCommand{ + ID: ds.ID, + OrgID: ds.OrgID, + Name: "test-datasource-updated", + SecureJsonData: map[string]string{ + expectedOgKey: expectedOgValue, + }, + } + + ds, err = dsService.UpdateDataSource(context.Background(), cmd) + require.NoError(t, err) + + secret, err := dsService.DecryptedValues(context.Background(), ds) + require.NoError(t, err) + + assert.Equal(t, secret[expectedDbKey], expectedDbValue) + assert.Equal(t, secret[expectedOgKey], expectedOgValue) + }) + + t.Run("should preserve cmd.SecureJsonData when cmd.IgnoreOldSecureJsonData=true", func(t *testing.T) { + sqlStore := db.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + quotaService := quotatest.New(false, nil) + mockPermission := acmock.NewMockedPermissionsService() + dsService, err := ProvideService(sqlStore, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), actest.FakeAccessControl{}, mockPermission, quotaService) + require.NoError(t, err) + + mockPermission.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil) + + notExpectedDbKey := "db-secure-key" + dbValue := "db-secure-value" + ds, err := dsService.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + OrgID: 1, + Name: "test-datasource", + SecureJsonData: map[string]string{ + notExpectedDbKey: dbValue, + }, + }) + require.NoError(t, err) + + expectedOgKey := "cmd-secure-key" + expectedOgValue := "cmd-secure-value" + + cmd := &datasources.UpdateDataSourceCommand{ + ID: ds.ID, + OrgID: ds.OrgID, + Name: "test-datasource-updated", + SecureJsonData: map[string]string{ + expectedOgKey: expectedOgValue, + }, + IgnoreOldSecureJsonData: true, + } + + ds, err = dsService.UpdateDataSource(context.Background(), cmd) + require.NoError(t, err) + + secret, err := dsService.DecryptedValues(context.Background(), ds) + require.NoError(t, err) + + assert.Equal(t, secret[expectedOgKey], expectedOgValue) + _, ok := secret[notExpectedDbKey] + assert.False(t, ok) + }) } func TestService_NameScopeResolver(t *testing.T) { diff --git a/pkg/services/provisioning/datasources/types.go b/pkg/services/provisioning/datasources/types.go index 69c7b0d7259..0ccaf68fe5c 100644 --- a/pkg/services/provisioning/datasources/types.go +++ b/pkg/services/provisioning/datasources/types.go @@ -242,21 +242,22 @@ func createUpdateCommand(ds *upsertDataSourceFromConfig, id int64) *datasources. } return &datasources.UpdateDataSourceCommand{ - ID: id, - UID: ds.UID, - OrgID: ds.OrgID, - Name: ds.Name, - Type: ds.Type, - Access: datasources.DsAccess(ds.Access), - URL: ds.URL, - User: ds.User, - Database: ds.Database, - BasicAuth: ds.BasicAuth, - BasicAuthUser: ds.BasicAuthUser, - WithCredentials: ds.WithCredentials, - IsDefault: ds.IsDefault, - JsonData: jsonData, - SecureJsonData: ds.SecureJSONData, - ReadOnly: !ds.Editable, + ID: id, + UID: ds.UID, + OrgID: ds.OrgID, + Name: ds.Name, + Type: ds.Type, + Access: datasources.DsAccess(ds.Access), + URL: ds.URL, + User: ds.User, + Database: ds.Database, + BasicAuth: ds.BasicAuth, + BasicAuthUser: ds.BasicAuthUser, + WithCredentials: ds.WithCredentials, + IsDefault: ds.IsDefault, + JsonData: jsonData, + SecureJsonData: ds.SecureJSONData, + ReadOnly: !ds.Editable, + IgnoreOldSecureJsonData: true, } }