From b25ea75bace4a4269d0bebe04c13bdc2986c165d Mon Sep 17 00:00:00 2001 From: Will Browne Date: Wed, 21 Sep 2022 12:20:11 +0200 Subject: [PATCH] Plugins: Add secure JSON fields to plugin setting DTO (#55313) * add secure JSON fields to plugin setting DTO * add nil pointer fix * adding secureJsonFields to the plugin meta. Co-authored-by: Marcus Andersson --- packages/grafana-data/src/types/plugin.ts | 1 + pkg/api/dtos/plugins.go | 25 +++++++------- pkg/api/fakes.go | 12 +++++-- pkg/api/frontendsettings.go | 25 +++++++------- pkg/api/plugins.go | 33 +++++++++++-------- pkg/models/plugin_settings.go | 8 +++++ .../service/dashboard_updater_test.go | 16 +++++++-- pkg/services/pluginsettings/models.go | 8 +++++ pkg/services/pluginsettings/pluginsettings.go | 2 +- .../pluginsettings/service/service.go | 26 +++++++-------- .../pluginsettings/service/service_test.go | 4 --- .../plugins/plugin_provisioner_test.go | 2 +- 12 files changed, 100 insertions(+), 62 deletions(-) diff --git a/packages/grafana-data/src/types/plugin.ts b/packages/grafana-data/src/types/plugin.ts index b940ab4ab4b..0d36d4ce3d9 100644 --- a/packages/grafana-data/src/types/plugin.ts +++ b/packages/grafana-data/src/types/plugin.ts @@ -69,6 +69,7 @@ export interface PluginMeta { // Filled in by the backend jsonData?: T; secureJsonData?: KeyValue; + secureJsonFields?: KeyValue; enabled?: boolean; defaultNavUrl?: string; hasUpdate?: boolean; diff --git a/pkg/api/dtos/plugins.go b/pkg/api/dtos/plugins.go index a7144b5b04a..ffb6b04c20a 100644 --- a/pkg/api/dtos/plugins.go +++ b/pkg/api/dtos/plugins.go @@ -6,18 +6,19 @@ import ( ) type PluginSetting struct { - Name string `json:"name"` - Type string `json:"type"` - Id string `json:"id"` - Enabled bool `json:"enabled"` - Pinned bool `json:"pinned"` - Module string `json:"module"` - BaseUrl string `json:"baseUrl"` - Info plugins.Info `json:"info"` - Includes []*plugins.Includes `json:"includes"` - Dependencies plugins.Dependencies `json:"dependencies"` - JsonData map[string]interface{} `json:"jsonData"` - DefaultNavUrl string `json:"defaultNavUrl"` + Name string `json:"name"` + Type string `json:"type"` + Id string `json:"id"` + Enabled bool `json:"enabled"` + Pinned bool `json:"pinned"` + Module string `json:"module"` + BaseUrl string `json:"baseUrl"` + Info plugins.Info `json:"info"` + Includes []*plugins.Includes `json:"includes"` + Dependencies plugins.Dependencies `json:"dependencies"` + JsonData map[string]interface{} `json:"jsonData"` + SecureJsonFields map[string]bool `json:"secureJsonFields"` + DefaultNavUrl string `json:"defaultNavUrl"` LatestVersion string `json:"latestVersion"` HasUpdate bool `json:"hasUpdate"` diff --git a/pkg/api/fakes.go b/pkg/api/fakes.go index c753d64583a..48c765a4a4e 100644 --- a/pkg/api/fakes.go +++ b/pkg/api/fakes.go @@ -86,10 +86,16 @@ type fakePluginSettings struct { } // GetPluginSettings returns all Plugin Settings for the provided Org -func (ps *fakePluginSettings) GetPluginSettings(ctx context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.DTO, error) { - res := []*pluginsettings.DTO{} +func (ps *fakePluginSettings) GetPluginSettings(_ context.Context, _ *pluginsettings.GetArgs) ([]*pluginsettings.InfoDTO, error) { + res := []*pluginsettings.InfoDTO{} for _, dto := range ps.plugins { - res = append(res, dto) + res = append(res, &pluginsettings.InfoDTO{ + PluginID: dto.PluginID, + OrgID: dto.OrgID, + Enabled: dto.Enabled, + Pinned: dto.Pinned, + PluginVersion: dto.PluginVersion, + }) } return res, nil } diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 20843b28200..77bf6cd76a0 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -431,8 +431,8 @@ func (hs *HTTPServer) enabledPlugins(ctx context.Context, orgID int64) (EnabledP return ep, nil } -func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[string]*pluginsettings.DTO, error) { - pluginSettings := make(map[string]*pluginsettings.DTO) +func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[string]*pluginsettings.InfoDTO, error) { + pluginSettings := make(map[string]*pluginsettings.InfoDTO) // fill settings from database if pss, err := hs.PluginSettings.GetPluginSettings(ctx, &pluginsettings.GetArgs{OrgID: orgID}); err != nil { @@ -451,11 +451,12 @@ func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[stri } // add new setting which is enabled depending on if AutoEnabled: true - pluginSetting := &pluginsettings.DTO{ - PluginID: plugin.ID, - OrgID: orgID, - Enabled: plugin.AutoEnabled, - Pinned: plugin.AutoEnabled, + pluginSetting := &pluginsettings.InfoDTO{ + PluginID: plugin.ID, + OrgID: orgID, + Enabled: plugin.AutoEnabled, + Pinned: plugin.AutoEnabled, + PluginVersion: plugin.Info.Version, } pluginSettings[plugin.ID] = pluginSetting @@ -469,10 +470,12 @@ func (hs *HTTPServer) pluginSettings(ctx context.Context, orgID int64) (map[stri } // add new setting which is enabled by default - pluginSetting := &pluginsettings.DTO{ - PluginID: plugin.ID, - OrgID: orgID, - Enabled: true, + pluginSetting := &pluginsettings.InfoDTO{ + PluginID: plugin.ID, + OrgID: orgID, + Enabled: true, + Pinned: false, + PluginVersion: plugin.Info.Version, } // if plugin is included in an app, check app settings diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index b6c4305aee7..30620b255c9 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -170,19 +170,20 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon } dto := &dtos.PluginSetting{ - Type: string(plugin.Type), - Id: plugin.ID, - Name: plugin.Name, - Info: plugin.Info, - Dependencies: plugin.Dependencies, - Includes: plugin.Includes, - BaseUrl: plugin.BaseURL, - Module: plugin.Module, - DefaultNavUrl: path.Join(hs.Cfg.AppSubURL, plugin.DefaultNavURL), - State: plugin.State, - Signature: plugin.Signature, - SignatureType: plugin.SignatureType, - SignatureOrg: plugin.SignatureOrg, + Type: string(plugin.Type), + Id: plugin.ID, + Name: plugin.Name, + Info: plugin.Info, + Dependencies: plugin.Dependencies, + Includes: plugin.Includes, + BaseUrl: plugin.BaseURL, + Module: plugin.Module, + DefaultNavUrl: path.Join(hs.Cfg.AppSubURL, plugin.DefaultNavURL), + State: plugin.State, + Signature: plugin.Signature, + SignatureType: plugin.SignatureType, + SignatureOrg: plugin.SignatureOrg, + SecureJsonFields: map[string]bool{}, } if plugin.IsApp() { @@ -202,6 +203,12 @@ func (hs *HTTPServer) GetPluginSettingByID(c *models.ReqContext) response.Respon dto.Enabled = ps.Enabled dto.Pinned = ps.Pinned dto.JsonData = ps.JSONData + + for k, v := range hs.PluginSettings.DecryptedValues(ps) { + if len(v) > 0 { + dto.SecureJsonFields[k] = true + } + } } update, exists := hs.pluginsUpdateChecker.HasUpdate(c.Req.Context(), plugin.ID) diff --git a/pkg/models/plugin_settings.go b/pkg/models/plugin_settings.go index 312f4601839..2052976917c 100644 --- a/pkg/models/plugin_settings.go +++ b/pkg/models/plugin_settings.go @@ -23,6 +23,14 @@ type PluginSetting struct { Updated time.Time } +type PluginSettingInfo struct { + PluginID string `xorm:"plugin_id"` + OrgID int64 `xorm:"org_id"` + Enabled bool `xorm:"enabled"` + Pinned bool `xorm:"pinned"` + PluginVersion string `xorm:"plugin_id"` +} + // ---------------------- // COMMANDS diff --git a/pkg/services/plugindashboards/service/dashboard_updater_test.go b/pkg/services/plugindashboards/service/dashboard_updater_test.go index 45402f6bb0b..a27a9ee4b65 100644 --- a/pkg/services/plugindashboards/service/dashboard_updater_test.go +++ b/pkg/services/plugindashboards/service/dashboard_updater_test.go @@ -397,9 +397,21 @@ type pluginsSettingsServiceMock struct { err error } -func (s *pluginsSettingsServiceMock) GetPluginSettings(_ context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.DTO, error) { +func (s *pluginsSettingsServiceMock) GetPluginSettings(_ context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.InfoDTO, error) { s.getPluginSettingsArgs = append(s.getPluginSettingsArgs, args.OrgID) - return s.storedPluginSettings, s.err + + var res []*pluginsettings.InfoDTO + for _, ps := range s.storedPluginSettings { + res = append(res, &pluginsettings.InfoDTO{ + PluginID: ps.PluginID, + OrgID: ps.OrgID, + Enabled: ps.Enabled, + Pinned: ps.Pinned, + PluginVersion: ps.PluginVersion, + }) + } + + return res, s.err } func (s *pluginsSettingsServiceMock) GetPluginSettingByPluginID(_ context.Context, args *pluginsettings.GetByPluginIDArgs) (*pluginsettings.DTO, error) { diff --git a/pkg/services/pluginsettings/models.go b/pkg/services/pluginsettings/models.go index d8e23326f2b..52fec91129a 100644 --- a/pkg/services/pluginsettings/models.go +++ b/pkg/services/pluginsettings/models.go @@ -16,6 +16,14 @@ type DTO struct { Updated time.Time } +type InfoDTO struct { + PluginID string + OrgID int64 + Enabled bool + Pinned bool + PluginVersion string +} + type UpdateArgs struct { Enabled bool Pinned bool diff --git a/pkg/services/pluginsettings/pluginsettings.go b/pkg/services/pluginsettings/pluginsettings.go index 7c8a3e94ff2..0abf9e81310 100644 --- a/pkg/services/pluginsettings/pluginsettings.go +++ b/pkg/services/pluginsettings/pluginsettings.go @@ -6,7 +6,7 @@ import ( type Service interface { // GetPluginSettings returns all Plugin Settings for the provided Org - GetPluginSettings(ctx context.Context, args *GetArgs) ([]*DTO, error) + GetPluginSettings(ctx context.Context, args *GetArgs) ([]*InfoDTO, error) // GetPluginSettingByPluginID returns a Plugin Settings by Plugin ID GetPluginSettingByPluginID(ctx context.Context, args *GetByPluginIDArgs) (*DTO, error) // UpdatePluginSetting updates a Plugin Setting diff --git a/pkg/services/pluginsettings/service/service.go b/pkg/services/pluginsettings/service/service.go index d735f1d7244..301f3bda239 100644 --- a/pkg/services/pluginsettings/service/service.go +++ b/pkg/services/pluginsettings/service/service.go @@ -44,24 +44,20 @@ type secureJSONDecryptionCache struct { sync.Mutex } -func (s *Service) GetPluginSettings(ctx context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.DTO, error) { - ps, err := s.getPluginSettings(ctx, args.OrgID) +func (s *Service) GetPluginSettings(ctx context.Context, args *pluginsettings.GetArgs) ([]*pluginsettings.InfoDTO, error) { + ps, err := s.getPluginSettingsInfo(ctx, args.OrgID) if err != nil { return nil, err } - var result []*pluginsettings.DTO + var result []*pluginsettings.InfoDTO for _, p := range ps { - result = append(result, &pluginsettings.DTO{ - ID: p.Id, - OrgID: p.OrgId, - PluginID: p.PluginId, - PluginVersion: p.PluginVersion, - JSONData: p.JsonData, - SecureJSONData: p.SecureJsonData, - Enabled: p.Enabled, - Pinned: p.Pinned, - Updated: p.Updated, + result = append(result, &pluginsettings.InfoDTO{ + OrgID: p.OrgID, + PluginID: p.PluginID, + PluginVersion: p.PluginVersion, + Enabled: p.Enabled, + Pinned: p.Pinned, }) } @@ -140,7 +136,7 @@ func (s *Service) DecryptedValues(ps *pluginsettings.DTO) map[string]string { return json } -func (s *Service) getPluginSettings(ctx context.Context, orgID int64) ([]*models.PluginSetting, error) { +func (s *Service) getPluginSettingsInfo(ctx context.Context, orgID int64) ([]*models.PluginSettingInfo, error) { sql := `SELECT org_id, plugin_id, enabled, pinned, plugin_version FROM plugin_setting ` params := make([]interface{}, 0) @@ -149,7 +145,7 @@ func (s *Service) getPluginSettings(ctx context.Context, orgID int64) ([]*models params = append(params, orgID) } - var rslt []*models.PluginSetting + var rslt []*models.PluginSettingInfo err := s.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { return sess.SQL(sql, params...).Find(&rslt) }) diff --git a/pkg/services/pluginsettings/service/service_test.go b/pkg/services/pluginsettings/service/service_test.go index 4e09fd84952..7edd10f5e31 100644 --- a/pkg/services/pluginsettings/service/service_test.go +++ b/pkg/services/pluginsettings/service/service_test.go @@ -137,8 +137,6 @@ func TestIntegrationPluginSettings(t *testing.T) { require.Equal(t, existing.OrgId, ps.OrgID) require.Equal(t, existing.PluginId, ps.PluginID) require.False(t, ps.Enabled) - require.Nil(t, ps.JSONData) - require.Nil(t, ps.SecureJSONData) }) t.Run("GetPluginSettings with orgID=1 should return all existing plugin settings", func(t *testing.T) { @@ -149,8 +147,6 @@ func TestIntegrationPluginSettings(t *testing.T) { require.Equal(t, existing.OrgId, ps.OrgID) require.Equal(t, existing.PluginId, ps.PluginID) require.False(t, ps.Enabled) - require.Nil(t, ps.JSONData) - require.Nil(t, ps.SecureJSONData) }) t.Run("GetPluginSettingById should return existing plugin settings", func(t *testing.T) { diff --git a/pkg/services/provisioning/plugins/plugin_provisioner_test.go b/pkg/services/provisioning/plugins/plugin_provisioner_test.go index e47f9d1da0f..0e2d6ccd34e 100644 --- a/pkg/services/provisioning/plugins/plugin_provisioner_test.go +++ b/pkg/services/provisioning/plugins/plugin_provisioner_test.go @@ -106,7 +106,7 @@ func (m *mockStore) UpdatePluginSettingPluginVersion(_ context.Context, _ *plugi return nil } -func (m *mockStore) GetPluginSettings(_ context.Context, _ *pluginsettings.GetArgs) ([]*pluginsettings.DTO, error) { +func (m *mockStore) GetPluginSettings(_ context.Context, _ *pluginsettings.GetArgs) ([]*pluginsettings.InfoDTO, error) { return nil, nil }