Plugins: Modify interface for plugin validations to allow taking PDC into account (#96089)

* Request interceptor: Do not block PDC

* Apply change after feedback received

* Add test

* Check if secure socks proxy configured for the instance

* Apply suggestions from code review

* Add dedicated service for datasource request URL validation (#99179)

---------

Co-authored-by: Will Browne <wbrowne@users.noreply.github.com>

pkg/api/datasources.go

@@ -841,12 +841,7 @@ func (hs *HTTPServer) checkDatasourceHealth(c *contextmodel.ReqContext, ds *data
 		Headers:       map[string]string{},
 	}
 
-	var dsURL string
+	err = hs.DataSourceRequestValidator.Validate(ds, c.Req)
-	if req.PluginContext.DataSourceInstanceSettings != nil {
-		dsURL = req.PluginContext.DataSourceInstanceSettings.URL
-	}
-
-	err = hs.PluginRequestValidator.Validate(dsURL, c.Req)
 	if err != nil {
 		return response.Error(http.StatusForbidden, "Access denied", err)
 	}

pkg/api/ds_query_test.go

@@ -36,7 +36,7 @@ import (
 	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
-type fakePluginRequestValidator struct {
+type fakeDataSourceRequestValidator struct {
 	err error
 }
 
@@ -45,7 +45,7 @@ type secretsErrorResponseBody struct {
 	Message string `json:"message"`
 }
 
-func (rv *fakePluginRequestValidator) Validate(dsURL string, req *http.Request) error {
+func (rv *fakeDataSourceRequestValidator) Validate(ds *datasources.DataSource, req *http.Request) error {
 	return rv.err
 }
 
@@ -56,7 +56,7 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) {
 		cfg,
 		nil,
 		nil,
-		&fakePluginRequestValidator{},
+		&fakeDataSourceRequestValidator{},
 		&fakePluginClient{
 			QueryDataHandlerFunc: func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
 				resp := backend.Responses{
@@ -114,7 +114,7 @@ func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) {
 		cfg,
 		nil,
 		nil,
-		&fakePluginRequestValidator{},
+		&fakeDataSourceRequestValidator{},
 		&fakePluginClient{
 			QueryDataHandlerFunc: func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
 				resp := backend.Responses{
@@ -309,7 +309,7 @@ func TestDataSourceQueryError(t *testing.T) {
 					cfg,
 					&fakeDatasources.FakeCacheService{},
 					nil,
-					&fakePluginRequestValidator{},
+					&fakeDataSourceRequestValidator{},
 					pluginClient.ProvideService(r),
 					plugincontext.ProvideService(cfg, localcache.ProvideService(), &pluginstore.FakePluginStore{
 						PluginList: []pluginstore.Plugin{pluginstore.ToGrafanaDTO(p)},

pkg/api/http_server.go

@@ -140,7 +140,7 @@ type HTTPServer struct {
 	License                      licensing.Licensing
 	AccessControl                accesscontrol.AccessControl
 	DataProxy                    *datasourceproxy.DataSourceProxyService
-	PluginRequestValidator       validations.PluginRequestValidator
+	DataSourceRequestValidator   validations.DataSourceRequestValidator
 	pluginClient                 plugins.Client
 	pluginStore                  pluginstore.Store
 	pluginInstaller              plugins.Installer
@@ -237,7 +237,7 @@ type ServerOptions struct {
 func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routing.RouteRegister, bus bus.Bus,
 	renderService rendering.Service, licensing licensing.Licensing, hooksService *hooks.HooksService,
 	cacheService *localcache.CacheService, sqlStore db.DB,
-	pluginRequestValidator validations.PluginRequestValidator, pluginStaticRouteResolver plugins.StaticRouteResolver,
+	dataSourceRequestValidator validations.DataSourceRequestValidator, pluginStaticRouteResolver plugins.StaticRouteResolver,
 	pluginDashboardService plugindashboards.Service, pluginStore pluginstore.Store, pluginClient plugins.Client,
 	pluginErrorResolver plugins.ErrorResolver, pluginInstaller plugins.Installer, settingsProvider setting.Provider,
 	dataSourceCache datasources.CacheService, userTokenService auth.UserTokenService,
@@ -284,7 +284,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		HooksService:                 hooksService,
 		CacheService:                 cacheService,
 		SQLStore:                     sqlStore,
-		PluginRequestValidator:       pluginRequestValidator,
+		DataSourceRequestValidator:   dataSourceRequestValidator,
 		pluginInstaller:              pluginInstaller,
 		pluginClient:                 pluginClient,
 		pluginStore:                  pluginStore,

pkg/api/plugin_resource.go

@@ -62,12 +62,7 @@ func (hs *HTTPServer) callPluginResourceWithDataSource(c *contextmodel.ReqContex
 		return
 	}
 
-	var dsURL string
+	err = hs.DataSourceRequestValidator.Validate(ds, c.Req)
-	if pCtx.DataSourceInstanceSettings != nil {
-		dsURL = pCtx.DataSourceInstanceSettings.URL
-	}
-
-	err = hs.PluginRequestValidator.Validate(dsURL, c.Req)
 	if err != nil {
 		c.JsonApiErr(http.StatusForbidden, "Access denied", err)
 		return

pkg/infra/httpclient/httpclientprovider/host_redirect_validation_middleware.go

@@ -11,7 +11,7 @@ import (
 
 const HostRedirectValidationMiddlewareName = "host-redirect-validation"
 
-func RedirectLimitMiddleware(reqValidator validations.PluginRequestValidator) sdkhttpclient.Middleware {
+func RedirectLimitMiddleware(reqValidator validations.DataSourceRequestURLValidator) sdkhttpclient.Middleware {
 	return sdkhttpclient.NamedMiddlewareFunc(HostRedirectValidationMiddlewareName, func(opts sdkhttpclient.Options, next http.RoundTripper) http.RoundTripper {
 		return sdkhttpclient.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
 			res, err := next.RoundTrip(req)
@@ -27,7 +27,7 @@ func RedirectLimitMiddleware(reqValidator validations.PluginRequestValidator) sd
 					return nil, locationErr
 				}
 
-				if validationErr := reqValidator.Validate(location.String(), nil); validationErr != nil {
+				if validationErr := reqValidator.Validate(location.String()); validationErr != nil {
 					return nil, validationErr
 				}
 			}

pkg/infra/httpclient/httpclientprovider/http_client_provider.go

@@ -20,7 +20,7 @@ import (
 var newProviderFunc = sdkhttpclient.NewProvider
 
 // New creates a new HTTP client provider with pre-configured middlewares.
-func New(cfg *setting.Cfg, validator validations.PluginRequestValidator, tracer tracing.Tracer) *sdkhttpclient.Provider {
+func New(cfg *setting.Cfg, validator validations.DataSourceRequestURLValidator, tracer tracing.Tracer) *sdkhttpclient.Provider {
 	logger := log.New("httpclient")
 
 	middlewares := []sdkhttpclient.Middleware{

pkg/infra/httpclient/httpclientprovider/http_client_provider_test.go

@@ -24,7 +24,7 @@ func TestHTTPClientProvider(t *testing.T) {
 			newProviderFunc = origNewProviderFunc
 		})
 		tracer := tracing.InitializeTracerForTest()
-		_ = New(&setting.Cfg{SigV4AuthEnabled: false}, &validations.OSSPluginRequestValidator{}, tracer)
+		_ = New(&setting.Cfg{SigV4AuthEnabled: false}, &validations.OSSDataSourceRequestURLValidator{}, tracer)
 		require.Len(t, providerOpts, 1)
 		o := providerOpts[0]
 		require.Len(t, o.Middlewares, 9)
@@ -50,7 +50,7 @@ func TestHTTPClientProvider(t *testing.T) {
 			newProviderFunc = origNewProviderFunc
 		})
 		tracer := tracing.InitializeTracerForTest()
-		_ = New(&setting.Cfg{SigV4AuthEnabled: true}, &validations.OSSPluginRequestValidator{}, tracer)
+		_ = New(&setting.Cfg{SigV4AuthEnabled: true}, &validations.OSSDataSourceRequestURLValidator{}, tracer)
 		require.Len(t, providerOpts, 1)
 		o := providerOpts[0]
 		require.Len(t, o.Middlewares, 10)
@@ -77,7 +77,7 @@ func TestHTTPClientProvider(t *testing.T) {
 			newProviderFunc = origNewProviderFunc
 		})
 		tracer := tracing.InitializeTracerForTest()
-		_ = New(&setting.Cfg{PluginSettings: setting.PluginSettings{"example": {"har_log_enabled": "true"}}}, &validations.OSSPluginRequestValidator{}, tracer)
+		_ = New(&setting.Cfg{PluginSettings: setting.PluginSettings{"example": {"har_log_enabled": "true"}}}, &validations.OSSDataSourceRequestURLValidator{}, tracer)
 		require.Len(t, providerOpts, 1)
 		o := providerOpts[0]
 		require.Len(t, o.Middlewares, 10)

pkg/server/wireexts_oss.go

@@ -70,7 +70,9 @@ var wireExtsBasicSet = wire.NewSet(
 	wire.Bind(new(pluginaccesscontrol.RoleRegistry), new(*acimpl.Service)),
 	wire.Bind(new(accesscontrol.Service), new(*acimpl.Service)),
 	validations.ProvideValidator,
-	wire.Bind(new(validations.PluginRequestValidator), new(*validations.OSSPluginRequestValidator)),
+	wire.Bind(new(validations.DataSourceRequestValidator), new(*validations.OSSDataSourceRequestValidator)),
+	validations.ProvideURLValidator,
+	wire.Bind(new(validations.DataSourceRequestURLValidator), new(*validations.OSSDataSourceRequestURLValidator)),
 	provisioning.ProvideService,
 	wire.Bind(new(provisioning.ProvisioningService), new(*provisioning.ProvisioningServiceImpl)),
 	backgroundsvcs.ProvideBackgroundServiceRegistry,

pkg/services/datasourceproxy/datasourceproxy.go

@@ -24,13 +24,13 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
-func ProvideService(dataSourceCache datasources.CacheService, plugReqValidator validations.PluginRequestValidator,
+func ProvideService(dataSourceCache datasources.CacheService, datasourceReqValidator validations.DataSourceRequestValidator,
 	pluginStore pluginstore.Store, cfg *setting.Cfg, httpClientProvider httpclient.Provider,
 	oauthTokenService *oauthtoken.Service, dsService datasources.DataSourceService,
 	tracer tracing.Tracer, secretsService secrets.Service, features featuremgmt.FeatureToggles) *DataSourceProxyService {
 	return &DataSourceProxyService{
 		DataSourceCache:            dataSourceCache,
-		PluginRequestValidator: plugReqValidator,
+		DataSourceRequestValidator: datasourceReqValidator,
 		pluginStore:                pluginStore,
 		Cfg:                        cfg,
 		HTTPClientProvider:         httpClientProvider,
@@ -44,7 +44,7 @@ func ProvideService(dataSourceCache datasources.CacheService, plugReqValidator v
 
 type DataSourceProxyService struct {
 	DataSourceCache            datasources.CacheService
-	PluginRequestValidator validations.PluginRequestValidator
+	DataSourceRequestValidator validations.DataSourceRequestValidator
 	pluginStore                pluginstore.Store
 	Cfg                        *setting.Cfg
 	HTTPClientProvider         httpclient.Provider
@@ -108,7 +108,7 @@ func toAPIError(c *contextmodel.ReqContext, err error) {
 }
 
 func (p *DataSourceProxyService) proxyDatasourceRequest(c *contextmodel.ReqContext, ds *datasources.DataSource) {
-	err := p.PluginRequestValidator.Validate(ds.URL, c.Req)
+	err := p.DataSourceRequestValidator.Validate(ds, c.Req)
 	if err != nil {
 		c.JsonApiErr(http.StatusForbidden, "Access denied", err)
 		return

pkg/services/datasourceproxy/datasourceproxy_test.go

@@ -94,7 +94,7 @@ func TestDatasourceProxy_proxyDatasourceRequest(t *testing.T) {
 			}}
 
 			p := DataSourceProxyService{
-				PluginRequestValidator: &fakePluginRequestValidator{},
+				DataSourceRequestValidator: &fakeDataSourceRequestValidator{},
 				pluginStore:                pluginStore,
 			}
 
@@ -129,8 +129,8 @@ func TestDatasourceProxy_proxyDatasourceRequest(t *testing.T) {
 	}
 }
 
-type fakePluginRequestValidator struct{}
+type fakeDataSourceRequestValidator struct{}
 
-func (rv *fakePluginRequestValidator) Validate(_ string, _ *http.Request) error {
+func (rv *fakeDataSourceRequestValidator) Validate(_ *datasources.DataSource, _ *http.Request) error {
 	return nil
 }

pkg/services/datasources/models.go

@@ -70,6 +70,16 @@ type DataSource struct {
 
 	Created time.Time `json:"created,omitempty"`
 	Updated time.Time `json:"updated,omitempty"`
+
+	isSecureSocksDSProxyEnabled *bool `xorm:"-"`
+}
+
+func (ds *DataSource) IsSecureSocksDSProxyEnabled() bool {
+	if ds.isSecureSocksDSProxyEnabled == nil {
+		enabled := ds.JsonData != nil && ds.JsonData.Get("enableSecureSocksProxy").MustBool(false)
+		ds.isSecureSocksDSProxyEnabled = &enabled
+	}
+	return *ds.isSecureSocksDSProxyEnabled
 }
 
 type TeamHTTPHeadersJSONData struct {

pkg/services/datasources/models_test.go

@@ -102,3 +102,58 @@ func TestTeamHTTPHeaders(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSecureSocksDSProxyEnabled(t *testing.T) {
+	testCases := []struct {
+		desc string
+		ds   *DataSource
+		want bool
+	}{
+		{
+			desc: "Empty json",
+			ds: &DataSource{
+				JsonData: simplejson.New(),
+			},
+			want: false,
+		},
+		{
+			desc: "Json with enableSecureSocksProxy",
+			ds: &DataSource{
+				JsonData: simplejson.NewFromAny(map[string]interface{}{
+					"enableSecureSocksProxy": true,
+				}),
+			},
+			want: true,
+		},
+		{
+			desc: "Json with string enableSecureSocksProxy",
+			ds: &DataSource{
+				JsonData: simplejson.NewFromAny(map[string]interface{}{
+					"enableSecureSocksProxy": "true",
+				}),
+			},
+			want: false,
+		},
+		{
+			desc: "Json with enableSecureSocksProxy false",
+			ds: &DataSource{
+				JsonData: simplejson.NewFromAny(map[string]interface{}{
+					"enableSecureSocksProxy": false,
+				}),
+			},
+			want: false,
+		},
+		{
+			desc: "Json with no json data",
+			ds:   &DataSource{},
+			want: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			actual := tc.ds.IsSecureSocksDSProxyEnabled()
+			assert.Equal(t, tc.want, actual)
+		})
+	}
+}

pkg/services/datasources/service/datasource.go

@@ -745,7 +745,7 @@ func (s *Service) httpClientOptions(ctx context.Context, ds *datasources.DataSou
 		}
 	}
 
-	if ds.JsonData != nil && ds.JsonData.Get("enableSecureSocksProxy").MustBool(false) {
+	if ds.IsSecureSocksDSProxyEnabled() {
 		proxyOpts := &sdkproxy.Options{
 			Enabled: true,
 			Auth: &sdkproxy.AuthOptions{

pkg/services/publicdashboards/api/common_test.go

@@ -152,18 +152,18 @@ func buildQueryDataService(t *testing.T, cs datasources.CacheService, fpc *fakeP
 		setting.NewCfg(),
 		cs,
 		nil,
-		&fakePluginRequestValidator{},
+		&fakeDataSourceRequestValidator{},
 		fpc,
 		pCtxProvider,
 	)
 }
 
 // copied from pkg/api/metrics_test.go
-type fakePluginRequestValidator struct {
+type fakeDataSourceRequestValidator struct {
 	err error
 }
 
-func (rv *fakePluginRequestValidator) Validate(dsURL string, req *http.Request) error {
+func (rv *fakeDataSourceRequestValidator) Validate(ds *datasources.DataSource, req *http.Request) error {
 	return rv.err
 }
 

pkg/services/query/query.go

@@ -42,7 +42,7 @@ func ProvideService(
 	cfg *setting.Cfg,
 	dataSourceCache datasources.CacheService,
 	expressionService *expr.Service,
-	pluginRequestValidator validations.PluginRequestValidator,
+	dataSourceRequestValidator validations.DataSourceRequestValidator,
 	pluginClient plugins.Client,
 	pCtxProvider *plugincontext.Provider,
 ) *ServiceImpl {
@@ -50,7 +50,7 @@ func ProvideService(
 		cfg:                        cfg,
 		dataSourceCache:            dataSourceCache,
 		expressionService:          expressionService,
-		pluginRequestValidator: pluginRequestValidator,
+		dataSourceRequestValidator: dataSourceRequestValidator,
 		pluginClient:               pluginClient,
 		pCtxProvider:               pCtxProvider,
 		log:                        log.New("query_data"),
@@ -73,7 +73,7 @@ type ServiceImpl struct {
 	cfg                        *setting.Cfg
 	dataSourceCache            datasources.CacheService
 	expressionService          *expr.Service
-	pluginRequestValidator validations.PluginRequestValidator
+	dataSourceRequestValidator validations.DataSourceRequestValidator
 	pluginClient               plugins.Client
 	pCtxProvider               *plugincontext.Provider
 	log                        log.Logger
@@ -244,7 +244,7 @@ func (s *ServiceImpl) handleExpressions(ctx context.Context, user identity.Reque
 func (s *ServiceImpl) handleQuerySingleDatasource(ctx context.Context, user identity.Requester, parsedReq *parsedRequest) (*backend.QueryDataResponse, error) {
 	queries := parsedReq.getFlattenedQueries()
 	ds := queries[0].datasource
-	if err := s.pluginRequestValidator.Validate(ds.URL, nil); err != nil {
+	if err := s.dataSourceRequestValidator.Validate(ds, nil); err != nil {
 		return nil, datasources.ErrDataSourceAccessDenied
 	}
 

pkg/services/query/query_test.go

@@ -462,7 +462,7 @@ func setup(t *testing.T) *testContext {
 	t.Helper()
 	pc := &fakePluginClient{}
 	dc := &fakeDataSourceCache{cache: dss}
-	rv := &fakePluginRequestValidator{}
+	rv := &fakeDataSourceRequestValidator{}
 
 	sqlStore, cfg := db.InitTestDBWithCfg(t)
 	secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -497,7 +497,7 @@ func setup(t *testing.T) *testContext {
 type testContext struct {
 	pluginContext          *fakePluginClient
 	secretStore            secretskvs.SecretsKVStore
-	pluginRequestValidator *fakePluginRequestValidator
+	pluginRequestValidator *fakeDataSourceRequestValidator
 	queryService           *ServiceImpl // implementation belonging to this package
 	signedInUser           *user.SignedInUser
 }
@@ -518,11 +518,11 @@ func metricRequestWithQueries(t *testing.T, rawQueries ...string) dtos.MetricReq
 	}
 }
 
-type fakePluginRequestValidator struct {
+type fakeDataSourceRequestValidator struct {
 	err error
 }
 
-func (rv *fakePluginRequestValidator) Validate(dsURL string, req *http.Request) error {
+func (rv *fakeDataSourceRequestValidator) Validate(ds *datasources.DataSource, req *http.Request) error {
 	return rv.err
 }
 

pkg/services/validations/oss.go

@@ -2,14 +2,26 @@ package validations
 
 import (
 	"net/http"
+
+	"github.com/grafana/grafana/pkg/services/datasources"
 )
 
-type OSSPluginRequestValidator struct{}
+type OSSDataSourceRequestValidator struct{}
+
+func (*OSSDataSourceRequestValidator) Validate(*datasources.DataSource, *http.Request) error {
+	return nil
+}
+
+func ProvideValidator() *OSSDataSourceRequestValidator {
+	return &OSSDataSourceRequestValidator{}
+}
+
+type OSSDataSourceRequestURLValidator struct{}
 
-func (*OSSPluginRequestValidator) Validate(string, *http.Request) error {
+func (*OSSDataSourceRequestURLValidator) Validate(string) error {
 	return nil
 }
 
-func ProvideValidator() *OSSPluginRequestValidator {
+func ProvideURLValidator() *OSSDataSourceRequestURLValidator {
-	return &OSSPluginRequestValidator{}
+	return &OSSDataSourceRequestURLValidator{}
 }

pkg/services/validations/service.go

@@ -2,11 +2,18 @@ package validations
 
 import (
 	"net/http"
+
+	"github.com/grafana/grafana/pkg/services/datasources"
 )
 
-type PluginRequestValidator interface {
+type DataSourceRequestValidator interface {
 	// Validate performs a request validation based
 	// on the data source URL and some of the request
 	// attributes (headers, cookies, etc).
-	Validate(dsURL string, req *http.Request) error
+	Validate(ds *datasources.DataSource, req *http.Request) error
+}
+
+type DataSourceRequestURLValidator interface {
+	// Validate performs a request validation based on the data source URL
+	Validate(dsURL string) error
 }