pkg/web: restrict handler types (#48495)

Makes `pkg/web` only accept handles from the following set:

```go
	handlerStd       = func(http.ResponseWriter, *http.Request)
	handlerStdCtx    = func(http.ResponseWriter, *http.Request, *web.Context)
	handlerStdReqCtx = func(http.ResponseWriter, *http.Request, *models.ReqContext)
	handlerReqCtx    = func(*models.ReqContext)
	handlerReqCtxRes = func(*models.ReqContext) Response
	handlerCtx       = func(*web.Context)
```

This is a first step to reducing above set to only `http.Handler`.

---

Due to a cyclic import situation between `pkg/models` and `pkg/web`, parts of this PR were put into `pkg/api/response`, even though they definitely do not belong there. This however is _temporary_ until we untangle `models.ReqContext`.

pkg/api/common_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/auth"
 	"github.com/grafana/grafana/pkg/services/contexthandler"
 	"github.com/grafana/grafana/pkg/services/contexthandler/authproxy"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	dashboardsstore "github.com/grafana/grafana/pkg/services/dashboards/database"
 	dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/service"
@@ -411,6 +412,9 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo
 		initCtx.Context = c
 		initCtx.Logger = log.New("api-test")
 		c.Map(initCtx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), initCtx))
+		c.Map(c.Req)
 	})
 
 	m.Use(accesscontrol.LoadPermissionsMiddleware(hs.AccessControl))

pkg/api/response/web_hack.go

@@ -0,0 +1,62 @@
+//nolint:unused,deadcode
+package response
+
+//NOTE: This file belongs into pkg/web, but due to cyclic imports that are hard to resolve at the current time, it temporarily lives here.
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
+	"github.com/grafana/grafana/pkg/web"
+)
+
+type (
+	handlerStd       = func(http.ResponseWriter, *http.Request)
+	handlerStdCtx    = func(http.ResponseWriter, *http.Request, *web.Context)
+	handlerStdReqCtx = func(http.ResponseWriter, *http.Request, *models.ReqContext)
+	handlerReqCtx    = func(*models.ReqContext)
+	handlerReqCtxRes = func(*models.ReqContext) Response
+	handlerCtx       = func(*web.Context)
+)
+
+func wrap_handler(h web.Handler) http.HandlerFunc {
+	switch handle := h.(type) {
+	case handlerStd:
+		return handle
+	case handlerStdCtx:
+		return func(w http.ResponseWriter, r *http.Request) {
+			handle(w, r, web.FromContext(r.Context()))
+		}
+	case handlerStdReqCtx:
+		return func(w http.ResponseWriter, r *http.Request) {
+			handle(w, r, getReqCtx(r.Context()))
+		}
+	case handlerReqCtx:
+		return func(w http.ResponseWriter, r *http.Request) {
+			handle(getReqCtx(r.Context()))
+		}
+	case handlerReqCtxRes:
+		return func(w http.ResponseWriter, r *http.Request) {
+			ctx := getReqCtx(r.Context())
+			res := handle(ctx)
+			res.WriteTo(ctx)
+		}
+	case handlerCtx:
+		return func(w http.ResponseWriter, r *http.Request) {
+			handle(web.FromContext(r.Context()))
+		}
+	}
+
+	panic(fmt.Sprintf("unexpected handler type: %T", h))
+}
+
+func getReqCtx(ctx context.Context) *models.ReqContext {
+	reqCtx, ok := ctx.Value(ctxkey.Key{}).(*models.ReqContext)
+	if !ok {
+		panic("no *models.ReqContext found")
+	}
+	return reqCtx
+}

pkg/infra/usagestats/service/api_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
@@ -103,5 +104,8 @@ func contextProvider(tc *testContext) web.Handler {
 			Logger:       log.New("test"),
 		}
 		c.Map(reqCtx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx))
+		c.Map(c.Req)
 	}
 }

pkg/services/accesscontrol/middleware_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -90,5 +91,8 @@ func contextProvider() web.Handler {
 			SkipCache:    true,
 		}
 		c.Map(reqCtx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx))
+		c.Map(c.Req)
 	}
 }

pkg/services/accesscontrol/resourcepermissions/api_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
@@ -500,6 +501,9 @@ func contextProvider(tc *testContext) web.Handler {
 			Logger:       log.New("test"),
 		}
 		c.Map(reqCtx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx))
+		c.Map(c.Req)
 	}
 }
 

pkg/services/contexthandler/contexthandler.go

@@ -19,6 +19,7 @@ import (
 	"github.com/grafana/grafana/pkg/middleware/cookies"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/contexthandler/authproxy"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/rendering"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -69,7 +70,7 @@ type ContextHandler struct {
 	GetTime func() time.Time
 }
 
-type reqContextKey struct{}
+type reqContextKey = ctxkey.Key
 
 // FromContext returns the ReqContext value stored in a context.Context, if any.
 func FromContext(c context.Context) *models.ReqContext {
@@ -94,7 +95,7 @@ func (h *ContextHandler) Middleware(mContext *web.Context) {
 	}
 
 	// Inject ReqContext into a request context and replace the request instance in the macaron context
-	mContext.Req = mContext.Req.WithContext(context.WithValue(mContext.Req.Context(), reqContextKey{}, reqContext))
+	mContext.Req = mContext.Req.WithContext(ctxkey.Set(mContext.Req.Context(), reqContext))
 	mContext.Map(mContext.Req)
 
 	traceID := tracing.TraceIDFromContext(mContext.Req.Context(), false)

pkg/services/contexthandler/ctxkey/key.go

@@ -0,0 +1,13 @@
+package ctxkey
+
+import "context"
+
+type Key struct{}
+
+func Set(ctx context.Context, data interface{}) context.Context {
+	return context.WithValue(ctx, Key{}, data)
+}
+
+func Get(ctx context.Context) interface{} {
+	return ctx.Value(Key{})
+}

pkg/services/ldap/testing.go

@@ -4,6 +4,9 @@ import (
 	"crypto/tls"
 
 	"gopkg.in/ldap.v3"
+
+	//TODO(sh0rez): remove once import cycle resolved
+	_ "github.com/grafana/grafana/pkg/api/response"
 )
 
 type searchFunc = func(request *ldap.SearchRequest) (*ldap.SearchResult, error)

pkg/services/multildap/multildap_test.go

@@ -8,6 +8,9 @@ import (
 	"github.com/grafana/grafana/pkg/services/ldap"
 
 	"github.com/stretchr/testify/require"
+
+	//TODO(sh0rez): remove once import cycle resolved
+	_ "github.com/grafana/grafana/pkg/api/response"
 )
 
 func TestMultiLDAP(t *testing.T) {

pkg/services/serviceaccounts/api/api_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
@@ -238,6 +239,9 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
 			Logger:       log.New("serviceaccounts-test"),
 		}
 		c.Map(ctx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), ctx))
+		c.Map(c.Req)
 	})
 	a.RouterRegister.Register(m.Router)
 	return m, a

pkg/web/macaron.go

@@ -19,9 +19,10 @@
 package web
 
 import (
+	_ "unsafe"
+
 	"context"
 	"net/http"
-	"reflect"
 	"strings"
 )
 
@@ -47,31 +48,14 @@ func Version() string {
 // and panics if an argument could not be fulfilled via dependency injection.
 type Handler interface{}
 
-// handlerFuncInvoker is an inject.FastInvoker wrapper of func(http.ResponseWriter, *http.Request).
+//go:linkname hack_wrap github.com/grafana/grafana/pkg/api/response.wrap_handler
-type handlerFuncInvoker func(http.ResponseWriter, *http.Request)
+func hack_wrap(Handler) http.HandlerFunc
-
-func (invoke handlerFuncInvoker) Invoke(params []interface{}) ([]reflect.Value, error) {
-	invoke(params[0].(http.ResponseWriter), params[1].(*http.Request))
-	return nil, nil
-}
 
 // validateAndWrapHandler makes sure a handler is a callable function, it panics if not.
 // When the handler is also potential to be any built-in inject.FastInvoker,
 // it wraps the handler automatically to have some performance gain.
 func validateAndWrapHandler(h Handler) Handler {
-	if reflect.TypeOf(h).Kind() != reflect.Func {
+	return hack_wrap(h)
-		panic("Macaron handler must be a callable function")
-	}
-
-	if !IsFastInvoker(h) {
-		switch v := h.(type) {
-		case func(*Context):
-			return ContextInvoker(v)
-		case func(http.ResponseWriter, *http.Request):
-			return handlerFuncInvoker(v)
-		}
-	}
-	return h
 }
 
 // validateAndWrapHandlers preforms validation and wrapping for each input handler.

pkg/web/macaron.s

@@ -0,0 +1,2 @@
+// ./hack/hack.go pushes its Wrap function as hack_wrap into this package.
+// This .s file is required to allow hack_wrap to be defined without having a function body.

pkg/web/webtest/webtest.go

@@ -11,6 +11,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/web"
 )
 
@@ -33,6 +34,9 @@ func NewServer(t testing.TB, routeRegister routing.RouteRegister) *Server {
 		initCtx.Context = c
 		initCtx.Logger = log.New("api-test")
 		c.Map(initCtx)
+
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), initCtx))
+		c.Map(c.Req)
 	})
 
 	m.Use(requestContextMiddleware())