public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Macaron: remove custom Request type (#37874)

Browse Source 
* remove macaron.Request, use http.Request instead

* remove com dependency from bindings module

* fix another c.Req.Request
This commit is contained in:
Serge Zaitsev
2021-09-01 11:18:30 +02:00
committed by GitHub
parent 56f61001a8
commit c3ab2fdeb7
35 changed files with 1205 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/app_routes.go
+1 -1
View File
@@ -59,6 +59,6 @@ func AppPluginRoute(route *plugins.AppPluginRoute, appID string, hs *HTTPServer)
proxy := pluginproxy.NewApiPluginProxy(c, path, route, appID, hs.Cfg)
proxy.Transport = pluginProxyTransport
proxy.ServeHTTP(c.Resp, c.Req.Request)
proxy.ServeHTTP(c.Resp, c.Req)
}
}
pkg/api/basic_auth.go
+2 -3
View File
@@ -2,15 +2,14 @@ package api
import (
"crypto/subtle"
macaron "gopkg.in/macaron.v1"
"net/http"
)
// BasicAuthenticatedRequest parses the provided HTTP request for basic authentication credentials
// and returns true if the provided credentials match the expected username and password.
// Returns false if the request is unauthenticated.
// Uses constant-time comparison in order to mitigate timing attacks.
func BasicAuthenticatedRequest(req macaron.Request, expectedUser, expectedPass string) bool {
func BasicAuthenticatedRequest(req *http.Request, expectedUser, expectedPass string) bool {
user, pass, ok := req.BasicAuth()
if !ok || subtle.ConstantTimeCompare([]byte(user), []byte(expectedUser)) != 1 || subtle.ConstantTimeCompare([]byte(pass), []byte(expectedPass)) != 1 {
return false
pkg/api/basic_auth_test.go
+2 -9
View File
@@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/macaron.v1"
)
func TestBasicAuthenticatedRequest(t *testing.T) {
@@ -16,11 +15,8 @@ func TestBasicAuthenticatedRequest(t *testing.T) {
const expectedPass = "password"
t.Run("Given a valid set of basic auth credentials", func(t *testing.T) {
httpReq, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil)
req, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil)
require.NoError(t, err)
req := macaron.Request{
Request: httpReq,
}
encodedCreds := encodeBasicAuthCredentials(expectedUser, expectedPass)
req.Header.Set("Authorization", fmt.Sprintf("Basic %s", encodedCreds))
authenticated := BasicAuthenticatedRequest(req, expectedUser, expectedPass)
@@ -29,11 +25,8 @@ func TestBasicAuthenticatedRequest(t *testing.T) {
})
t.Run("Given an invalid set of basic auth credentials", func(t *testing.T) {
httpReq, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil)
req, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil)
require.NoError(t, err)
req := macaron.Request{
Request: httpReq,
}
encodedCreds := encodeBasicAuthCredentials("invaliduser", "invalidpass")
req.Header.Set("Authorization", fmt.Sprintf("Basic %s", encodedCreds))
authenticated := BasicAuthenticatedRequest(req, expectedUser, expectedPass)
pkg/api/dashboard_test.go
+1 -1
View File
@@ -33,7 +33,7 @@ import (
func TestGetHomeDashboard(t *testing.T) {
httpReq, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
req := &models.ReqContext{SignedInUser: &models.SignedInUser{}, Context: &macaron.Context{Req: macaron.Request{Request: httpReq}}}
req := &models.ReqContext{SignedInUser: &models.SignedInUser{}, Context: &macaron.Context{Req: httpReq}}
cfg := setting.NewCfg()
cfg.StaticRootPath = "../../public/"
pkg/api/grafana_com_proxy.go
+1 -1
View File
@@ -44,6 +44,6 @@ func ProxyGnetRequest(c *models.ReqContext) {
proxyPath := c.Params("*")
proxy := ReverseProxyGnetReq(proxyPath)
proxy.Transport = grafanaComProxyTransport
proxy.ServeHTTP(c.Resp, c.Req.Request)
proxy.ServeHTTP(c.Resp, c.Req)
c.Resp.Header().Del("Set-Cookie")
}
pkg/api/http_server.go
+1 -1
View File
@@ -457,7 +457,7 @@ func (hs *HTTPServer) metricsEndpoint(ctx *macaron.Context) {
promhttp.
HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}).
ServeHTTP(ctx.Resp, ctx.Req.Request)
ServeHTTP(ctx.Resp, ctx.Req)
}
// healthzHandler always return 200 - Ok if Grafana's web server is running
pkg/api/pluginproxy/ds_proxy.go
+13 -13
View File
@@ -147,7 +147,7 @@ func (proxy *DataSourceProxy) HandleRequest() {
span, ctx := opentracing.StartSpanFromContext(proxy.ctx.Req.Context(), "datasource reverse proxy")
defer span.Finish()
proxy.ctx.Req.Request = proxy.ctx.Req.WithContext(ctx)
proxy.ctx.Req = proxy.ctx.Req.WithContext(ctx)
span.SetTag("datasource_name", proxy.ds.Name)
span.SetTag("datasource_type", proxy.ds.Type)
@@ -160,11 +160,11 @@ func (proxy *DataSourceProxy) HandleRequest() {
if err := opentracing.GlobalTracer().Inject(
span.Context(),
opentracing.HTTPHeaders,
opentracing.HTTPHeadersCarrier(proxy.ctx.Req.Request.Header)); err != nil {
opentracing.HTTPHeadersCarrier(proxy.ctx.Req.Header)); err != nil {
logger.Error("Failed to inject span context instance", "err", err)
}
reverseProxy.ServeHTTP(proxy.ctx.Resp, proxy.ctx.Req.Request)
reverseProxy.ServeHTTP(proxy.ctx.Resp, proxy.ctx.Req)
}
func (proxy *DataSourceProxy) addTraceFromHeaderValue(span opentracing.Span, headerName string, tagName string) {
@@ -252,13 +252,13 @@ func (proxy *DataSourceProxy) validateRequest() error {
}
if proxy.ds.Type == models.DS_ES {
if proxy.ctx.Req.Request.Method == "DELETE" {
if proxy.ctx.Req.Method == "DELETE" {
return errors.New("deletes not allowed on proxied Elasticsearch datasource")
}
if proxy.ctx.Req.Request.Method == "PUT" {
if proxy.ctx.Req.Method == "PUT" {
return errors.New("puts not allowed on proxied Elasticsearch datasource")
}
if proxy.ctx.Req.Request.Method == "POST" && proxy.proxyPath != "_msearch" {
if proxy.ctx.Req.Method == "POST" && proxy.proxyPath != "_msearch" {
return errors.New("posts not allowed on proxied Elasticsearch datasource except on /_msearch")
}
}
@@ -289,13 +289,13 @@ func (proxy *DataSourceProxy) validateRequest() error {
// Trailing validation below this point for routes that were not matched
if proxy.ds.Type == models.DS_PROMETHEUS {
if proxy.ctx.Req.Request.Method == "DELETE" {
if proxy.ctx.Req.Method == "DELETE" {
return errors.New("non allow-listed DELETEs not allowed on proxied Prometheus datasource")
}
if proxy.ctx.Req.Request.Method == "PUT" {
if proxy.ctx.Req.Method == "PUT" {
return errors.New("non allow-listed PUTs not allowed on proxied Prometheus datasource")
}
if proxy.ctx.Req.Request.Method == "POST" {
if proxy.ctx.Req.Method == "POST" {
return errors.New("non allow-listed POSTs not allowed on proxied Prometheus datasource")
}
}
@@ -309,10 +309,10 @@ func (proxy *DataSourceProxy) logRequest() {
}
var body string
if proxy.ctx.Req.Request.Body != nil {
buffer, err := ioutil.ReadAll(proxy.ctx.Req.Request.Body)
if proxy.ctx.Req.Body != nil {
buffer, err := ioutil.ReadAll(proxy.ctx.Req.Body)
if err == nil {
proxy.ctx.Req.Request.Body = ioutil.NopCloser(bytes.NewBuffer(buffer))
proxy.ctx.Req.Body = ioutil.NopCloser(bytes.NewBuffer(buffer))
body = string(buffer)
}
}
@@ -323,7 +323,7 @@ func (proxy *DataSourceProxy) logRequest() {
"username", proxy.ctx.Login,
"datasource", proxy.ds.Type,
"uri", proxy.ctx.Req.RequestURI,
"method", proxy.ctx.Req.Request.Method,
"method", proxy.ctx.Req.Method,
"body", body)
}
pkg/api/pluginproxy/ds_proxy_test.go
+9 -25
View File
@@ -103,9 +103,7 @@ func TestDataSourceProxy_routeRule(t *testing.T) {
req, err := http.NewRequest("GET", "http://localhost/asd", nil)
require.NoError(t, err)
ctx := &models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR},
}
return ctx, req
@@ -239,9 +237,7 @@ func TestDataSourceProxy_routeRule(t *testing.T) {
req, err := http.NewRequest("GET", "http://localhost/asd", nil)
require.NoError(t, err)
ctx := &models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR},
}
@@ -450,9 +446,7 @@ func TestDataSourceProxy_routeRule(t *testing.T) {
require.NoError(t, err)
ctx := &models.ReqContext{
SignedInUser: &models.SignedInUser{UserId: 1},
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
}
mockAuthToken := mockOAuthTokenService{
token: &oauth2.Token{
@@ -582,9 +576,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
return &models.ReqContext{
SignedInUser: &models.SignedInUser{},
Context: &macaron.Context{
Req: macaron.Request{
Request: httptest.NewRequest("GET", "/render", nil),
},
Req: httptest.NewRequest("GET", "/render", nil),
Resp: responseWriter,
},
}, ds
@@ -647,7 +639,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
},
})
ctx.Req.Request = httptest.NewRequest("GET", "/api/datasources/proxy/1/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", nil)
ctx.Req = httptest.NewRequest("GET", "/api/datasources/proxy/1/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", nil)
proxy, err := NewDataSourceProxy(ds, plugin, ctx, "/path/%2Ftest%2Ftest%2F", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{})
require.NoError(t, err)
@@ -661,9 +653,7 @@ func TestDataSourceProxy_requestHandling(t *testing.T) {
func TestNewDataSourceProxy_InvalidURL(t *testing.T) {
ctx := models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{},
},
Context: &macaron.Context{},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR},
}
ds := models.DataSource{
@@ -679,9 +669,7 @@ func TestNewDataSourceProxy_InvalidURL(t *testing.T) {
func TestNewDataSourceProxy_ProtocolLessURL(t *testing.T) {
ctx := models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{},
},
Context: &macaron.Context{},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR},
}
ds := models.DataSource{
@@ -699,9 +687,7 @@ func TestNewDataSourceProxy_ProtocolLessURL(t *testing.T) {
// Test wth MSSQL type data sources.
func TestNewDataSourceProxy_MSSQL(t *testing.T) {
ctx := models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{},
},
Context: &macaron.Context{},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_EDITOR},
}
tcs := []struct {
@@ -899,9 +885,7 @@ func Test_PathCheck(t *testing.T) {
req, err := http.NewRequest("GET", "http://localhost/asd", nil)
require.NoError(t, err)
ctx := &models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
SignedInUser: &models.SignedInUser{OrgRole: models.ROLE_VIEWER},
}
return ctx, req
pkg/api/plugins.go
+1 -1
View File
@@ -312,7 +312,7 @@ func (hs *HTTPServer) GetPluginAssets(c *models.ReqContext) {
c.Resp.Header().Set("Cache-Control", "public, max-age=3600")
}
http.ServeContent(c.Resp, c.Req.Request, pluginFilePath, fi.ModTime(), f)
http.ServeContent(c.Resp, c.Req, pluginFilePath, fi.ModTime(), f)
}
// CheckHealth returns the health of a plugin.
pkg/api/render.go
+1 -1
View File
@@ -76,5 +76,5 @@ func (hs *HTTPServer) RenderToPng(c *models.ReqContext) {
}
c.Resp.Header().Set("Content-Type", "image/png")
http.ServeFile(c.Resp, c.Req.Request, result.FilePath)
http.ServeFile(c.Resp, c.Req, result.FilePath)
}
pkg/api/static/static.go
+2 -2
View File
@@ -160,7 +160,7 @@ func staticHandler(ctx *macaron.Context, log log.Logger, opt StaticOptions) bool
rePrefix := regexp.MustCompile(`^(?:/\\|/+)`)
path = rePrefix.ReplaceAllString(path, "/")
}
http.Redirect(ctx.Resp, ctx.Req.Request, path, http.StatusFound)
http.Redirect(ctx.Resp, ctx.Req, path, http.StatusFound)
return true
}
@@ -190,7 +190,7 @@ func staticHandler(ctx *macaron.Context, log log.Logger, opt StaticOptions) bool
opt.AddHeaders(ctx)
}
http.ServeContent(ctx.Resp, ctx.Req.Request, file, fi.ModTime(), f)
http.ServeContent(ctx.Resp, ctx.Req, file, fi.ModTime(), f)
return true
}
pkg/api/team_test.go
+2 -6
View File
@@ -126,9 +126,7 @@ func TestTeamAPIEndpoint(t *testing.T) {
t.Run("with no real signed in user", func(t *testing.T) {
stub := &testLogger{}
c := &models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
SignedInUser: &models.SignedInUser{},
Logger: stub,
}
@@ -144,9 +142,7 @@ func TestTeamAPIEndpoint(t *testing.T) {
t.Run("with real signed in user", func(t *testing.T) {
stub := &testLogger{}
c := &models.ReqContext{
Context: &macaron.Context{
Req: macaron.Request{Request: req},
},
Context: &macaron.Context{Req: req},
SignedInUser: &models.SignedInUser{UserId: 42},
Logger: stub,
}