diff --git a/pkg/api/dataproxy.go b/pkg/api/dataproxy.go index 290ed274541..4dcea44b5bd 100644 --- a/pkg/api/dataproxy.go +++ b/pkg/api/dataproxy.go @@ -1,6 +1,9 @@ package api import ( + "errors" + "fmt" + "github.com/grafana/grafana/pkg/api/pluginproxy" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/models" @@ -32,7 +35,15 @@ func (hs *HTTPServer) ProxyDataSourceRequest(c *models.ReqContext) { // macaron does not include trailing slashes when resolving a wildcard path proxyPath := ensureProxyPathTrailingSlash(c.Req.URL.Path, c.Params("*")) - proxy := pluginproxy.NewDataSourceProxy(ds, plugin, c, proxyPath, hs.Cfg) + proxy, err := pluginproxy.NewDataSourceProxy(ds, plugin, c, proxyPath, hs.Cfg) + if err != nil { + if errors.Is(err, pluginproxy.URLValidationError{}) { + c.JsonApiErr(400, fmt.Sprintf("Invalid data source URL: %q", ds.Url), err) + } else { + c.JsonApiErr(500, "Failed creating data source proxy", err) + } + return + } proxy.HandleRequest() } diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index c56562219c3..3b2f18c00f5 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -2,6 +2,8 @@ package api import ( "encoding/json" + "fmt" + "net/url" "sort" "github.com/grafana/grafana/pkg/api/dtos" @@ -10,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/util/errutil" ) func GetDataSources(c *models.ReqContext) Response { @@ -124,8 +127,23 @@ func DeleteDataSourceByName(c *models.ReqContext) Response { return Success("Data source deleted") } +func validateURL(u string) Response { + if u != "" { + _, err := url.Parse(u) + if err != nil { + return Error(400, fmt.Sprintf("Validation error, invalid URL: %q", u), errutil.Wrapf(err, + "invalid data source URL %q", u)) + } + } + + return nil +} + func AddDataSource(c *models.ReqContext, cmd models.AddDataSourceCommand) Response { cmd.OrgId = c.OrgId + if resp := validateURL(cmd.Url); resp != nil { + return resp + } if err := bus.Dispatch(&cmd); err != nil { if err == models.ErrDataSourceNameExists || err == models.ErrDataSourceUidExists { @@ -147,6 +165,9 @@ func AddDataSource(c *models.ReqContext, cmd models.AddDataSourceCommand) Respon func UpdateDataSource(c *models.ReqContext, cmd models.UpdateDataSourceCommand) Response { cmd.OrgId = c.OrgId cmd.Id = c.ParamsInt64(":id") + if resp := validateURL(cmd.Url); resp != nil { + return resp + } err := fillWithSecureJSONData(&cmd) if err != nil { diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index a4eae6794cf..d1b4ef8b5f0 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -31,6 +31,20 @@ var ( client = newHTTPClient() ) +type URLValidationError struct { + error + + url string +} + +func (e URLValidationError) Error() string { + return fmt.Sprintf("Validation of URL %q failed: %s", e.url, e.error.Error()) +} + +func (e URLValidationError) Unwrap() error { + return e.error +} + type DataSourceProxy struct { ds *models.DataSource ctx *models.ReqContext @@ -70,8 +84,12 @@ func (lw *logWrapper) Write(p []byte) (n int, err error) { } // NewDataSourceProxy creates a new Datasource proxy -func NewDataSourceProxy(ds *models.DataSource, plugin *plugins.DataSourcePlugin, ctx *models.ReqContext, proxyPath string, cfg *setting.Cfg) *DataSourceProxy { - targetURL, _ := url.Parse(ds.Url) +func NewDataSourceProxy(ds *models.DataSource, plugin *plugins.DataSourcePlugin, ctx *models.ReqContext, + proxyPath string, cfg *setting.Cfg) (*DataSourceProxy, error) { + targetURL, err := url.Parse(ds.Url) + if err != nil { + return nil, URLValidationError{error: err, url: ds.Url} + } return &DataSourceProxy{ ds: ds, @@ -80,7 +98,7 @@ func NewDataSourceProxy(ds *models.DataSource, plugin *plugins.DataSourcePlugin, proxyPath: proxyPath, targetUrl: targetURL, cfg: cfg, - } + }, nil } func newHTTPClient() httpClient { diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 509d015329f..2b78a2aa645 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -7,11 +7,14 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" "github.com/grafana/grafana/pkg/components/securejsondata" "github.com/grafana/grafana/pkg/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/oauth2" macaron "gopkg.in/macaron.v1" @@ -86,7 +89,8 @@ func TestDSRouteRule(t *testing.T) { } Convey("When matching route path", func() { - proxy := NewDataSourceProxy(ds, plugin, ctx, "api/v4/some/method", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "api/v4/some/method", &setting.Cfg{}) + So(err, ShouldBeNil) proxy.route = plugin.Routes[0] ApplyRoute(proxy.ctx.Req.Context(), req, proxy.proxyPath, proxy.route, proxy.ds) @@ -97,7 +101,8 @@ func TestDSRouteRule(t *testing.T) { }) Convey("When matching route path and has dynamic url", func() { - proxy := NewDataSourceProxy(ds, plugin, ctx, "api/common/some/method", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "api/common/some/method", &setting.Cfg{}) + So(err, ShouldBeNil) proxy.route = plugin.Routes[3] ApplyRoute(proxy.ctx.Req.Context(), req, proxy.proxyPath, proxy.route, proxy.ds) @@ -109,21 +114,24 @@ func TestDSRouteRule(t *testing.T) { Convey("Validating request", func() { Convey("plugin route with valid role", func() { - proxy := NewDataSourceProxy(ds, plugin, ctx, "api/v4/some/method", &setting.Cfg{}) - err := proxy.validateRequest() + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "api/v4/some/method", &setting.Cfg{}) + So(err, ShouldBeNil) + err = proxy.validateRequest() So(err, ShouldBeNil) }) Convey("plugin route with admin role and user is editor", func() { - proxy := NewDataSourceProxy(ds, plugin, ctx, "api/admin", &setting.Cfg{}) - err := proxy.validateRequest() + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "api/admin", &setting.Cfg{}) + So(err, ShouldBeNil) + err = proxy.validateRequest() So(err, ShouldNotBeNil) }) Convey("plugin route with admin role and user is admin", func() { ctx.SignedInUser.OrgRole = models.ROLE_ADMIN - proxy := NewDataSourceProxy(ds, plugin, ctx, "api/admin", &setting.Cfg{}) - err := proxy.validateRequest() + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "api/admin", &setting.Cfg{}) + So(err, ShouldBeNil) + err = proxy.validateRequest() So(err, ShouldBeNil) }) }) @@ -191,7 +199,8 @@ func TestDSRouteRule(t *testing.T) { So(err, ShouldBeNil) client = newFakeHTTPClient(json) - proxy1 := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken1", &setting.Cfg{}) + proxy1, err := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken1", &setting.Cfg{}) + So(err, ShouldBeNil) proxy1.route = plugin.Routes[0] ApplyRoute(proxy1.ctx.Req.Context(), req, proxy1.proxyPath, proxy1.route, proxy1.ds) @@ -205,7 +214,8 @@ func TestDSRouteRule(t *testing.T) { req, _ := http.NewRequest("GET", "http://localhost/asd", nil) client = newFakeHTTPClient(json2) - proxy2 := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken2", &setting.Cfg{}) + proxy2, err := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken2", &setting.Cfg{}) + So(err, ShouldBeNil) proxy2.route = plugin.Routes[1] ApplyRoute(proxy2.ctx.Req.Context(), req, proxy2.proxyPath, proxy2.route, proxy2.ds) @@ -220,7 +230,8 @@ func TestDSRouteRule(t *testing.T) { req, _ := http.NewRequest("GET", "http://localhost/asd", nil) client = newFakeHTTPClient([]byte{}) - proxy3 := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken1", &setting.Cfg{}) + proxy3, err := NewDataSourceProxy(ds, plugin, ctx, "pathwithtoken1", &setting.Cfg{}) + So(err, ShouldBeNil) proxy3.route = plugin.Routes[0] ApplyRoute(proxy3.ctx.Req.Context(), req, proxy3.proxyPath, proxy3.route, proxy3.ds) @@ -241,7 +252,8 @@ func TestDSRouteRule(t *testing.T) { ds := &models.DataSource{Url: "htttp://graphite:8080", Type: models.DS_GRAPHITE} ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + So(err, ShouldBeNil) req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) So(err, ShouldBeNil) @@ -266,7 +278,8 @@ func TestDSRouteRule(t *testing.T) { } ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + So(err, ShouldBeNil) req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) So(err, ShouldBeNil) @@ -290,7 +303,8 @@ func TestDSRouteRule(t *testing.T) { } ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + So(err, ShouldBeNil) requestURL, _ := url.Parse("http://grafana.com/sub") req := http.Request{URL: requestURL, Header: make(http.Header)} @@ -316,7 +330,8 @@ func TestDSRouteRule(t *testing.T) { } ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "", &setting.Cfg{}) + So(err, ShouldBeNil) requestURL, _ := url.Parse("http://grafana.com/sub") req := http.Request{URL: requestURL, Header: make(http.Header)} @@ -337,7 +352,8 @@ func TestDSRouteRule(t *testing.T) { Url: "http://host/root/", } ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(ds, plugin, ctx, "/path/to/folder/", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/path/to/folder/", &setting.Cfg{}) + So(err, ShouldBeNil) req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) req.Header.Add("Origin", "grafana.com") req.Header.Add("Referer", "grafana.com") @@ -393,9 +409,9 @@ func TestDSRouteRule(t *testing.T) { Req: macaron.Request{Request: req}, }, } - proxy := NewDataSourceProxy(ds, plugin, ctx, "/path/to/folder/", &setting.Cfg{}) - req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) - + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/path/to/folder/", &setting.Cfg{}) + So(err, ShouldBeNil) + req, err = http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) So(err, ShouldBeNil) proxy.getDirector()(req) @@ -505,7 +521,8 @@ func TestDSRouteRule(t *testing.T) { Convey("When response header Set-Cookie is not set should remove proxied Set-Cookie header", func() { writeErr = nil ctx := setupCtx(nil) - proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + So(err, ShouldBeNil) proxy.HandleRequest() @@ -518,7 +535,8 @@ func TestDSRouteRule(t *testing.T) { ctx := setupCtx(func(w http.ResponseWriter) { w.Header().Set("Set-Cookie", "important_cookie=important_value") }) - proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) + So(err, ShouldBeNil) proxy.HandleRequest() @@ -530,6 +548,24 @@ func TestDSRouteRule(t *testing.T) { }) } +func TestNewDataSourceProxy_InvalidURL(t *testing.T) { + ctx := models.ReqContext{ + Context: &macaron.Context{ + Req: macaron.Request{}, + }, + SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR}, + } + ds := models.DataSource{ + Type: "test", + Url: "://host/root", + } + cfg := setting.Cfg{} + plugin := plugins.DataSourcePlugin{} + _, err := NewDataSourceProxy(&ds, &plugin, &ctx, "api/method", &cfg) + require.Error(t, err) + assert.True(t, strings.HasPrefix(err.Error(), `Validation of URL "://host/root" failed`)) +} + type CloseNotifierResponseRecorder struct { *httptest.ResponseRecorder closeChan chan bool @@ -553,7 +589,8 @@ func getDatasourceProxiedRequest(ctx *models.ReqContext, cfg *setting.Cfg) *http Url: "http://host/root/", } - proxy := NewDataSourceProxy(ds, plugin, ctx, "", cfg) + proxy, err := NewDataSourceProxy(ds, plugin, ctx, "", cfg) + So(err, ShouldBeNil) req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) So(err, ShouldBeNil) @@ -662,7 +699,8 @@ func createAuthTest(dsType string, authType string, authCheck string, useSecureJ func runDatasourceAuthTest(test *Test) { plugin := &plugins.DataSourcePlugin{} ctx := &models.ReqContext{} - proxy := NewDataSourceProxy(test.datasource, plugin, ctx, "", &setting.Cfg{}) + proxy, err := NewDataSourceProxy(test.datasource, plugin, ctx, "", &setting.Cfg{}) + So(err, ShouldBeNil) req, err := http.NewRequest(http.MethodGet, "http://grafana.com/sub", nil) So(err, ShouldBeNil)